search

terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
224 stars 87 forks source link

Check on snapshot DB name is not smart #1724

Open keckler opened 3 months ago

keckler commented 3 months ago

There is a settings validator that checks the name of the database being used in a snapshot run: https://github.com/terrapower/armi/blob/f9903fe5a5d30ceda389d6494020f7a0c0f01b92/armi/operators/settingsValidation.py#L450-L459

This check tries to ensure that the snapshot DB and the current run name are not the same, presumably so that the old DB doesn't get overwritten.

This check does not account for the fact that the snapshot DB might be in a different directory, in which case there wouldn't be any issue and the warning is misguided.

john-science commented 3 months ago

This check does not account for the fact that the snapshot DB might be in a different directory, in which case there wouldn't be any issue and the warning is misguided.

Doesn't it?

In this line:

https://github.com/terrapower/armi/blob/f9903fe5a5d30ceda389d6494020f7a0c0f01b92/armi/operators/settingsValidation.py#L452

The os.path.basename() gets the file name from the path.

john-science commented 3 months ago

We discussed, and our suggested change goes something like this:

  1. Check the reloadDB path is not in the current directory, and that it is not a child current/working dir
    • (It would be nice if we could guarantee the case suite child naming.)
  2. It can be in the current/working dir, if it has a different name than the caseTitle (this is the current check)

The only thing I'm not sure about is if we can get the "current working directory" safely in call cases in this lambda function.