-
Notifications
You must be signed in to change notification settings - Fork 97
Refactoring loadTestReactor to pickle multiple test reactors #2334
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
base: main
Are you sure you want to change the base?
Conversation
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.
Oh this is great! I especially love that pickle is now being spelled properly
We know this gives a speedup, but we need to monitor memory too since we are pickling so many reactors (depending on the repo). I will run testing tomorrow for memory use.
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.
Obvi we need to wait to merge due to downstream impacts, but I enthusiastically approve of this approach. In the meantime, a couple of comments.
|
This looks great! We have to wait a week to merge so I'll come give a green check when we are ready. But I don't see any more changes needed unless something unexpected shows up in downstream testing. |
What is the change? Why is it being made?
Essentially, when
loadTestReactor()was first written, there was only ONE test reactor in ARMI. But now there are all kinds of them in ARMI and in downstream ARMI projects.This PR helps cache/pickle all such test reactors. Though it provides a method to not do that, say for instance if you know that a certain test reactor will only ever be used in one test. You can save the RAM by not pickling it.
close #2333
close #1636
SCR Information
Change Type: trivial
One-Sentence Description: Refactoring loadTestReactor to pickle multiple test reactors
One-line Impact on Requirements: NA
Checklist
docfolder.pyproject.toml.