-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: rename vis_cpu to matvis #72
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 567 566 -1
Branches 88 88
=========================================
- Hits 567 566 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @piyanatk. This all looks good. My only comment is that I think we should split the methods at the module level, rather than the function-name level. I think this is better because it allows us to easily swap in different methods by just adding a new module. It is more along the lines of how other "drop-in replacement" libraries work (e.g. jax.numpy
etc).
I think the GPU tests are failing either because the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @piyanatk !
This PR rename
vis_cpu
tomatvis
following the consensus from the paper. It will also require refactoring inhera_sim