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
Use Random123 instead of Isaac64, ACG and MLCG #2674
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2674 +/- ##
==========================================
+ Coverage 66.23% 66.26% +0.02%
==========================================
Files 559 557 -2
Lines 104008 103950 -58
==========================================
- Hits 68890 68881 -9
+ Misses 35118 35069 -49 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
✔️ af57bd1 -> Azure artifacts URL |
✔️ a5507cc -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fd61b20
to
2fc44a4
Compare
This comment has been minimized.
This comment has been minimized.
@alkino : I was reviewing Michael's comment during last meeting (recording here @ time 17:20). He confirmed removal of 1) Isaac64 2) ACG 3) MLCG. So I was thinking the steps could be following:
So considering the above, I am wondering if it is possible to split the removal of the above 3 RNs in one PR and MCellRan4 into another one. My "hope" is that the removal of the above 3 should be straightforward / less or no CI failures and hence can be merged faster. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✔️ efbf7c5 -> Azure artifacts URL |
#include "Normal.h" | ||
|
||
Rand::Rand(unsigned long seed, int size, Object* obj) { | ||
// printf("Rand\n"); | ||
gen = new ACG(seed, size); | ||
gen = new NrnRandom123(seed, size); |
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.
A bit weird, what do you think about that?
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.
not sure implications. lets discuss with Michael.
This comment has been minimized.
This comment has been minimized.
✔️ f48accb -> Azure artifacts URL |
Quality Gate passedIssues Measures |
✔️ 76d3d1d -> Azure artifacts URL |
Remove usage of:
Isaac64
ACG
MLCG