-
Notifications
You must be signed in to change notification settings - Fork 86
add magic with reverse norm order #797
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
add magic with reverse norm order #797
Conversation
Codecov ReportBase: 94.78% // Head: 94.92% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #797 +/- ##
==========================================
+ Coverage 94.78% 94.92% +0.14%
==========================================
Files 155 157 +2
Lines 4163 4279 +116
Branches 215 227 +12
==========================================
+ Hits 3946 4062 +116
Misses 142 142
Partials 75 75
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
wes-lewis
left a comment
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.
All changes look reasonable, and this addresses the current issue of including both "reversed order" and standard order of normalizations
|
Mind converting your review to an approval instead of a comment so I can merge? Thanks! |
It appears that one of the denoising:Tabula Muris tests failed, but I'm not sure if the error that is shown (137) is a flake. I'm use to looking at github actions rather than NextFlow to gauge whether tests have passed. I'm inclined to merge unless this is seen as a significant issue. Ed: the overall workflow is passing. Individual jobs failing for 137 is an OOM which is saved by rerunning with more memory. |
Reversing the order of sqrt and libnorm inexplicably improves performance. Adding this to MAGIC with explicit naming so users are aware that this is different from defaults.