Skip to content

Improve test coverage for rewrite package#2240

Closed
ReneEnjilian wants to merge 16 commits intoapache:mainfrom
ReneEnjilian:rewrite-algebraic
Closed

Improve test coverage for rewrite package#2240
ReneEnjilian wants to merge 16 commits intoapache:mainfrom
ReneEnjilian:rewrite-algebraic

Conversation

@ReneEnjilian
Copy link
Contributor

This pull request is the first in a series aimed at enhancing the test coverage of the rewrite package. Specifically, it adds additional tests for the algebraic simplifications defined in RewriteAlgebraicSimplificationStatic. The goal is also to proactively identify potential bugs, similar to our previous efforts. No bugs were found in this patch.

@mboehm7
Copy link
Contributor

mboehm7 commented Mar 2, 2025

Thanks @ReneEnjilian - could you please address the failing rewrite tests (I just kicked them off again, but there seem to be systematic failures).

@ReneEnjilian
Copy link
Contributor Author

I implemented a fix and hope that this resolves the issue now. The error does not occur on my machine but from what I can see is that R struggled with the class type of the result matrix. Changing outer(v, 1:m, "==") to outer(as.vector(v), 1:m, "==") should resolve the error because R is now recognized as type matrix and not only as type array anymore.

@codecov
Copy link

codecov bot commented Mar 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.46%. Comparing base (78b23cf) to head (27f09d8).
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2240      +/-   ##
============================================
- Coverage     72.46%   72.46%   -0.01%     
- Complexity    45453    45462       +9     
============================================
  Files          1469     1469              
  Lines        170893   170893              
  Branches      33325    33325              
============================================
- Hits         123846   123845       -1     
- Misses        37630    37640      +10     
+ Partials       9417     9408       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mboehm7
Copy link
Contributor

mboehm7 commented Mar 3, 2025

LGTM - thanks @ReneEnjilian for the effort improving our test coverage.

@mboehm7 mboehm7 closed this in cfbe190 Mar 3, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in SystemDS PR Queue Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants