Organize output files by cycle/node. #952
Conversation
ARMI can a large number of input files at each cycle and node. Most of these file names are re-used at different cycle/node points, so they get overwritten. To improve organization and retain all important inputs and outputs, the directory changer is being updated to copy these files to a permanent location in the working directly, under folders named c0n0, c0n1, etc.
|
Yes, IT is going to love this! 😆 |
Ideally it will be easier to delete old files you don't want because they'll be grouped into a handful of folders, rather than just having 1000+ loose files in the working directory. For now we still are copying to the working directory so the files are there for codes to interface with each other, but hopefully we can get to a point where the physics codes can find the files they need within these directories. |
john-science
added
feature request
|
Why did you change the |
|
@jakehader Please do not merge this yet. @mgjarrett Have you tested this downstream in our other important repos and projects? It appears to me that most of the projects that use ARMI to run simulations would have to change to meet your new change. I would like you to propose this change at the next Monday meeting, and spearhead the work in what I am guessing is about 12 other pull requests in other repositories. Thanks so much! |
There are downstream repos that use the |
The goal of this PR is to retain all physics kernel outputs from a simulation in a location where they will not be overwritten at the next time step. I would like to get input from others at the next meeting. I am not in any rush for this to be merged, and it will be important to get collective agreement about how this should be implemented. The hope is that we can improve the organization of the many, many physics kernel outputs that are typically generated during an ARMI run. Right now, you might just have 1000+ files sitting loose in a directory with no organization. No downstream projects need to change in order to work with this proposed PR, in its current form. The intent is to add the option for anyone using a |
|
@mgjarrett Can you verify something for me? Something that would be Bad would be if running 2 simulations in a row could now result in over-writing the first results because the second results are in the same folder. That's not the case with your re-design here, right? Have you tested that? |
Return `dumpSnapshot` closer to its original purpose of dumping the physics kernel inputs and outputs without performing any detailed analysis. Moved the existing functionality of `dumpSnapshot` to a new setting called `runSnapshot`, which is a clearer description of what the setting does: run detailed analysis at the specified "snapshots" (i.e., state points).
This is the current behavior of ARMI, and this PR does not change that. If you re-run ARMI in the same folder where you have previously run it, it will overwrite any existing files as new ones are generated. The benefit of this PR is that it allows you to store inputs/outputs from a given time step in the simulation. Previously, any auxiliary I/O from a physics kernel would be overwritten unless it was given a name with a unique cycle/step identifier. |
|
After our internal user meeting, @ntouran brought up that what we are implementing here is basically the old behavior of the I am proposing with this PR to change the name of the current With this change, we can reclaim the To summarize:
For example: will dump I/O at |
|
This looks great to me! Holding on any approvals and landing until we get the greenlight from @john-science |
|
Sorry, I can't tell, did we get rid of the "dumpSnapshots" setting in this PR? It looks like we did, but I thought we were keeping that and adding a new level of snapshot detail? (I just mention it because I quickly found 9 downstream repos that use that setting. So, even just changing the spelling of that name would generate a lot of work that feels unnecessary if we can leave the name unchanged.) |
We can change the names to whatever we want. I am suggesting these names because |
I am proposing renaming it to
We're not adding the detailed calculation; the current setting
Yes, we'd have to update the name of the setting in downstream repos. They're trivial PRs, but it is unnecessary work. I am arguing that the current setting name is confusing because it doesn't just dump data, it also runs additional calculations. But that might not outweigh the burden of changing a setting name. If that's the case, we can come up with another name for what this PR is doing. Maybe |
|
@mgjarrett Okay, this PR has gone on too long, sorry about that. @ntouran Agrees though that we should either: a. Leave the Nick thinks you're probably right, that we should just rename the setting. |
|
Okay, I am happy to change the settings names to whatever we all agree on. For the new setting being implemented here, what is the favored name? I threw out I'll leave |
It's up to you. It's your baby. I'm not sure
The only thing we need is for the name (and the |
Okay, I'm hesitant to use
|
Description
ARMI can output a large number of input files at each cycle and node. Most of these file names are re-used at different cycle/node points, so they get overwritten. To improve organization and retain all important inputs and outputs, the directory changer is being updated to copy these files to a permanent location in the working directly, under folders named
c0n0,c0n1, etc, using a new settingsavePhysicsFiles.Note
This PR does not change the functionality of
dumpSnapshot, which is now used to run detailed physics calculations at selected time points.Checklist
doc/release/0.X.rst) are up-to-date with any bug fixes or new features.docfolder.setup.py.