Repository: terrapower/armi
Pull request #1416 (2023-09-26 00:21:09)

copy interface inputs to return relative paths
## What is the change?

This update is twofold:

1. Copy Interface Inputs now returns a relative file path when a file is copied rather than an absolute file path
2. Moves warnings about file copy to DEBUG 

## Why is the change being made?

1. When the directory structure changes, a settings file will no longer be valid. This prevents that
2. A lot of warnings print with case suites that have dependencies because the files don't yet exist. This moves those print outs to a more verbose run log level for user convenience.

Resolves Issue #1304 


---

## Checklist

<!--
    You (the pull requester) should put an `x` in the boxes below you have completed.
    If you're unsure about any of them, don't hesitate to ask. We're here to help!
    Learn what a "good PR" looks like here:
    https://terrapower.github.io/armi/developer/tooling.html#good-pull-requests
-->

- [x] This PR has only one purpose or idea.
- [x] Tests have been added/updated to verify that the new/changed code works.

<!-- Check the code quality -->

- [x] The code style follows [good practices](https://terrapower.github.io/armi/developer/standards_and_practices.html).
- [x] The commit message(s) follow [good practices](https://terrapower.github.io/armi/developer/tooling.html).

<!-- Check the project-level cruft -->

- [x] The [release notes](https://terrapower.github.io/armi/release/index.html) (location `doc/release/0.X.rst`) are up-to-date with any important changes.
- [x] The documentation is still up-to-date in the `doc` folder.
- [x] The dependencies are still up-to-date in `setup.py`.


opotowsky: @john-science could you see this impacting anyone's workflow externally? Internally this is both tested and improves our workflow. This can be merged immediately if we don't think we need to warn anyone about this update. 
john-science: I merged this branch with `main`, and handled the merge conflicts.

I will probably merge this PR as soon as the CI passes again.

Patch:
diff --git a/armi/cases/case.py b/armi/cases/case.py
index 7ad72532..e42879e6 100644
--- a/armi/cases/case.py
+++ b/armi/cases/case.py
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-r"""
+"""
 The ``Case`` object is responsible for running, and executing a set of user inputs.  Many
 entry points redirect into ``Case`` methods, such as ``clone``, ``compare``, and ``run``.
 
@@ -872,8 +872,11 @@ def _copyInputsHelper(
     try:
         pathTools.copyOrWarn(fileDescription, sourcePath, destFilePath)
         if pathlib.Path(destFilePath).exists():
-            return destFilePath
+            # the basename gets written back to the settings file to protect against
+            # potential future dir structure changes
+            return os.path.basename(destFilePath)
         else:
+            # keep original filepath in the settings file if file copy was unsuccessful
             return origFile
     except Exception:
         return origFile
@@ -961,6 +964,7 @@ def copyInterfaceInputs(
                             continue
                     except OSError:
                         pass
+
                 # Attempt to construct an absolute file path
                 sourceFullPath = os.path.join(sourceDirPath, f)
                 if WILDCARD:
@@ -982,11 +986,11 @@ def copyInterfaceInputs(
                         label, sourceFullPath, destination, f
                     )
                     newFiles.append(str(destFilePath))
+
                 if destFilePath == f:
-                    runLog.info(
+                    runLog.debug(
                         f"No input files for `{label}` setting could be resolved with "
-                        f"the following path: `{sourceFullPath}`. Will not update "
-                        f"`{label}`."
+                        f"the following path: `{sourceFullPath}`. Will not update `{label}`."
                     )
 
             # Some settings are a single filename. Others are lists of files. Make
@@ -995,4 +999,5 @@ def copyInterfaceInputs(
                 newSettings[label] = newFiles[0]
             else:
                 newSettings[label] = newFiles
+
     return newSettings
diff --git a/armi/operators/settingsValidation.py b/armi/operators/settingsValidation.py
index ebab2cbc..e9b4ad37 100644
--- a/armi/operators/settingsValidation.py
+++ b/armi/operators/settingsValidation.py
@@ -338,7 +338,7 @@ class Inspector:
         against the default, if the user specifies all the simple cycle settings
         _exactly_ as the defaults, this won't be caught. But, it would be very
         coincidental for the user to _specify_ all the default values when
-        performing any real analysis, so whatever.
+        performing any real analysis.
 
         Also, we must bypass the `Settings` getter and reach directly
         into the underlying `__settings` dict to avoid triggering an error
@@ -718,7 +718,7 @@ class Inspector:
 
 
 def createQueryRevertBadPathToDefault(inspector, settingName, initialLambda=None):
-    r"""
+    """
     Return a query to revert a bad path to its default.
 
     Parameters
diff --git a/armi/physics/neutronics/settings.py b/armi/physics/neutronics/settings.py
index b181234e..45841188 100644
--- a/armi/physics/neutronics/settings.py
+++ b/armi/physics/neutronics/settings.py
@@ -155,6 +155,7 @@ def defineSettings():
                 "ZeroInwardCurrent",
                 "Generalized",
             ],
+            enforcedOptions=True,
         ),
         setting.Setting(
             CONF_NEUTRONICS_KERNEL,
diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst
index 5316f669..e1d0402e 100644
--- a/doc/release/0.2.rst
+++ b/doc/release/0.2.rst
@@ -8,6 +8,7 @@ Release Date: TBD
 
 What's new in ARMI
 ------------------
+#. The ``_copyInputsHelper()`` gives relative path and not absolute after copy. (`PR#1416 <https://github.com/terrapower/armi/pull/1416>`_)
 #. TBD
 
 Bug fixes


Test patch:
diff --git a/armi/cases/tests/test_cases.py b/armi/cases/tests/test_cases.py
index 94356c21..f030c507 100644
--- a/armi/cases/tests/test_cases.py
+++ b/armi/cases/tests/test_cases.py
@@ -505,8 +505,9 @@ class TestCopyInterfaceInputs(unittest.TestCase):
                 destPath=newDir.destination,
                 origFile=shuffleFile,
             )
-            newFilepath = os.path.join(newDir.destination, shuffleFile)
-            self.assertEqual(destFilePath, str(newFilepath))
+            newFilePath = os.path.join(newDir.destination, shuffleFile)
+            self.assertTrue(os.path.exists(newFilePath))
+            self.assertEqual(destFilePath, os.path.basename(newFilePath))
 
         # test with bad file path, should return original file
         # ensure we are not in TEST_ROOT
@@ -514,9 +515,10 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             destFilePath = cases.case._copyInputsHelper(
                 testSetting,
                 sourcePath=sourceFullPath,
-                destPath="",
+                destPath="fakeDest",
                 origFile=shuffleFile,
             )
+            self.assertFalse(os.path.exists(destFilePath))
             self.assertEqual(destFilePath, shuffleFile)
 
     def test_copyInterfaceInputs_singleFile(self):
@@ -529,8 +531,9 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
-            newFilepath = os.path.join(newDir.destination, shuffleFile)
-            self.assertEqual(newSettings[testSetting], str(newFilepath))
+            newFilePath = os.path.join(newDir.destination, shuffleFile)
+            self.assertTrue(os.path.exists(newFilePath))
+            self.assertEqual(newSettings[testSetting], os.path.basename(newFilePath))
 
     def test_copyInterfaceInputs_nonFilePath(self):
         testSetting = CONF_SHUFFLE_LOGIC
@@ -543,6 +546,7 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
+            self.assertFalse(os.path.exists(newSettings[testSetting]))
             self.assertEqual(newSettings[testSetting], fakeShuffle)
 
     def test_copyInterfaceInputs_multipleFiles(self):
@@ -569,8 +573,10 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
-            newFilepaths = [os.path.join(newDir.destination, f) for f in settingFiles]
-            self.assertEqual(newSettings[testSetting], newFilepaths)
+            newFilePaths = [os.path.join(newDir.destination, f) for f in settingFiles]
+            for newFilePath in newFilePaths:
+                self.assertTrue(os.path.exists(newFilePath))
+            self.assertEqual(newSettings[testSetting], settingFiles)
 
     def test_copyInterfaceInputs_wildcardFile(self):
         testSetting = CONF_SHUFFLE_LOGIC
@@ -584,8 +590,11 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
-            newFilepath = [os.path.join(newDir.destination, "ISOAA")]
-            self.assertEqual(newSettings[testSetting], newFilepath)
+            newFilePath = [os.path.join(newDir.destination, "ISOAA")]
+            self.assertTrue(os.path.exists(newFilePath[0]))
+            self.assertEqual(
+                newSettings[testSetting], [os.path.basename(newFilePath[0])]
+            )
 
         # Check on a file that doesn't exist (so globFilePaths len is 0)
         wcFile = "fakeFile*"
@@ -594,6 +603,7 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
+            self.assertFalse(os.path.exists(newSettings[testSetting][0]))
             self.assertEqual(newSettings[testSetting], [wcFile])
 
     def test_copyInterfaceInputs_relPath(self):
@@ -608,8 +618,9 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
-            newFilepath = os.path.join(newDir.destination, shuffleFile)
-            self.assertEqual(newSettings[testSetting], newFilepath)
+            newFilePath = os.path.join(newDir.destination, shuffleFile)
+            self.assertTrue(os.path.exists(newFilePath))
+            self.assertEqual(newSettings[testSetting], os.path.basename(newFilePath))
 
     def test_copyInterfaceInputs_absPath(self):
         testSetting = CONF_SHUFFLE_LOGIC
@@ -623,4 +634,8 @@ class TestCopyInterfaceInputs(unittest.TestCase):
             newSettings = cases.case.copyInterfaceInputs(
                 cs, destination=newDir.destination
             )
+            # file exists
+            self.assertTrue(os.path.exists(newSettings[testSetting]))
+            # but not copied to this dir
+            self.assertFalse(os.path.exists(os.path.basename(newSettings[testSetting])))
             self.assertEqual(str(newSettings[testSetting]), absFile)
diff --git a/armi/physics/neutronics/tests/test_neutronicsPlugin.py b/armi/physics/neutronics/tests/test_neutronicsPlugin.py
index 8b79919b..dbeac4fc 100644
--- a/armi/physics/neutronics/tests/test_neutronicsPlugin.py
+++ b/armi/physics/neutronics/tests/test_neutronicsPlugin.py
@@ -264,8 +264,8 @@ class NeutronicsReactorTests(unittest.TestCase):
         sv = getNeutronicsSettingValidators(inspector)
         self.assertEqual(len(sv), 9)
 
-        # Test the Query: boundaries are now "Extrapolated", not "Normal"
-        cs = cs.modified(newSettings={CONF_BOUNDARIES: "Normal"})
+        # Test the Query: boundaries are now "Extrapolated", not "Generalized"
+        cs = cs.modified(newSettings={CONF_BOUNDARIES: "Generalized"})
         inspector = settingsValidation.Inspector(cs)
         sv = getNeutronicsSettingValidators(inspector)