-
Notifications
You must be signed in to change notification settings - Fork 89
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 new memory runLog info to memoryProfiler.py #1970
Conversation
@@ -101,11 +129,30 @@ def interactEveryNode(self, cycle, node): | |||
mpiAction.broadcast().invoke(self.o, self.r, self.cs) | |||
|
|||
def interactEOL(self): | |||
r"""End of life hook. Good place to wrap up or print out summary outputs.""" | |||
"""End of life hook. Good place to wrap up or print out summary outputs.""" | |||
if self.cs["debugMem"]: |
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.
I'm just wondering... should this same settings check be done at interactBOL
and interactEveryNode
?
I could be totally off-base here, I just can't tell why this is only done at the one time node.
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.
I'm not following. Can you elaborate?
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.
Well, this memory profiler does nothing in interactEOL
unless the setting debugMem
is set to True
.
So.... I would EXPECT that setting would control all the time points, not just the one.
If that's not the case cool, I'm just wondering what the difference is.
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.
That setting does get used in interactBOL
and interactEveryNode
. These two just also do other stuff too.
Granted I don't know the original intent. The git blame for the vast majority of this module go well beyond my time with TerraPower.
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.
Approved! I have a couple of minor thoughts for you, but no hills I'm prepared to die on.
Co-authored-by: John Stilley <[email protected]>
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.
I'm a fan!
Note that we realize this code probably shouldn't be in ARMI but to push towards a deadline we have a separate cleanup ticket to bring parts of the memory profiler into internal tooling, where it belongs.
* main: Fixing HexBlock docstrings (#1981) Removing a duplicate hex rotation impl tag (#1979) Avoiding closing plots that are meant to be interactive (#1978) Ensuring HexBlock.rotate updates child spatial locators (#1943) Add new memory runLog info to memoryProfiler.py (#1970) Fixing various doc build issues (#1974) Hiding sphinx-needs warnings during doc build (#1973) Fixing warnings in CLI startup (#1972)
What is the change?
Adds some potentially useful
runLog
statements to the memory profiler interface on how much memory is is use , how much is available, and the total available for a simulation. The first two are dynamic (i.e., change with (cycle, node)) while the third is static.Why is the change being made?
Some memory intensive downstream analysis found this useful, so we thought we'd push it up to ARMI.
Checklist
doc
folder.pyproject.toml
.