Repository: terrapower/armi

Issue:
Force Parameters to have descriptions
When constructing a `Parameter` object, we already have multiple checks to ensure the `Parameter` is valid:

https://github.com/terrapower/armi/blob/59388d29f8929178e0e1aa4d8f0f160095a2b739/armi/reactor/parameters/parameterDefinitions.py#L250-L253

Why not add some to ensure there is a valid `description` field?  I want to say the same for `units`, but many parameters are unitless, so we would have to change "unitless" to be a non-empty string.

```python
assert description is not None and len(description)
```

> After all, what good is a `Parameter` if no one can tell what it is?

Discussion:

Patch:
diff --git a/armi/cases/case.py b/armi/cases/case.py
index c111866c..0b733335 100644
--- a/armi/cases/case.py
+++ b/armi/cases/case.py
@@ -618,32 +618,6 @@ class Case:
         """Uses the ReportInterface to create a fancy HTML page describing the design inputs."""
         _ = reportsEntryPoint.createReportFromSettings(self.cs)
 
-    def buildCommand(self, python="python"):
-        """
-        Build an execution command for running or submitting a job.
-
-        Parameters
-        ----------
-        python : str, optional
-            The path to the python executable to use for executing the case. By default
-            this will be whatever "python" resolves to in the target environment.
-            However when running in more exotic environments (e.g. HPC cluster), it is
-            usually desireable to provide a specific python executable.
-        """
-        command = ""
-        if self.cs["numProcessors"] > 1:
-            command += "mpiexec -n {} ".format(self.cs["numProcessors"])
-            if self.cs["mpiTasksPerNode"] > 0:
-                command += "-c {} ".format(self.cs["mpiTasksPerNode"])
-
-        command += "{} -u ".format(python)
-        if not __debug__:
-            command += " -O "
-
-        command += ' -m {} run "{}.yaml"'.format(context.APP_NAME, self.cs.caseTitle)
-
-        return command
-
     def clone(
         self,
         additionalFiles=None,
diff --git a/armi/context.py b/armi/context.py
index 6b6492d4..5f4205d0 100644
--- a/armi/context.py
+++ b/armi/context.py
@@ -95,7 +95,10 @@ START_TIME = time.ctime()
 
 # Set batch mode if not a TTY, which means you're on a cluster writing to a stdout file
 # In this mode you cannot respond to prompts or anything
-CURRENT_MODE = Mode.INTERACTIVE if sys.stdout.isatty() else Mode.BATCH
+# This does not work replabliy for both windows and linux so an os-specific solution is
+# applied
+isatty = sys.stdout.isatty() if "win" in sys.platform else sys.stdin.isatty()
+CURRENT_MODE = Mode.INTERACTIVE if isatty else Mode.BATCH
 Mode.setMode(CURRENT_MODE)
 
 MPI_COMM = None
@@ -125,10 +128,6 @@ try:
     MPI_SIZE = MPI_COMM.Get_size()
     MPI_NODENAME = MPI.Get_processor_name()
     MPI_NODENAMES = MPI_COMM.allgather(MPI_NODENAME)
-
-    # fix an exceptional error case when we are not in "interactive mode"
-    if MPI_SIZE > 1 and CURRENT_MODE == Mode.INTERACTIVE:
-        CURRENT_MODE = Mode.BATCH
 except ImportError:
     # stick with defaults
     pass
diff --git a/armi/physics/neutronics/globalFlux/globalFluxInterface.py b/armi/physics/neutronics/globalFlux/globalFluxInterface.py
index 3f2272b7..3f43fe19 100644
--- a/armi/physics/neutronics/globalFlux/globalFluxInterface.py
+++ b/armi/physics/neutronics/globalFlux/globalFluxInterface.py
@@ -33,7 +33,7 @@ from armi.reactor.converters import geometryConverters
 from armi.reactor.converters import uniformMesh
 from armi.reactor.flags import Flags
 from armi.settings.caseSettings import Settings
-from armi.utils import units, codeTiming, getMaxBurnSteps
+from armi.utils import units, codeTiming, getMaxBurnSteps, getBurnSteps
 
 ORDER = interfaces.STACK_ORDER.FLUX
 
@@ -85,7 +85,7 @@ class GlobalFluxInterface(interfaces.Interface):
 
     def interactBOC(self, cycle=None):
         interfaces.Interface.interactBOC(self, cycle)
-        self.r.core.p.rxSwing = 0.0  # zero out rxSwing until EOC.
+        self.r.core.p.rxSwing = 0.0  # zero out rxSwing until last time node.
         self.r.core.p.maxDetailedDpaThisCycle = 0.0  # zero out cumulative params
         self.r.core.p.dpaFullWidthHalfMax = 0.0
         self.r.core.p.elevationOfACLP3Cycles = 0.0
@@ -102,23 +102,30 @@ class GlobalFluxInterface(interfaces.Interface):
         is up to date with the reactor state.
         """
         interfaces.Interface.interactEveryNode(self, cycle, node)
-
-        if self.r.p.timeNode == 0:
-            self._bocKeff = self.r.core.p.keff  # track boc keff for rxSwing param.
+        self._setRxSwingRelatedParams()
 
     def interactCoupled(self, iteration):
         """Runs during a tightly-coupled physics iteration to updated the flux and power."""
         interfaces.Interface.interactCoupled(self, iteration)
+        self._setRxSwingRelatedParams()
+
+    def _setRxSwingRelatedParams(self):
+        """Set Params Related to Rx Swing."""
         if self.r.p.timeNode == 0:
-            self._bocKeff = self.r.core.p.keff  # track boc keff for rxSwing param.
-
-    def interactEOC(self, cycle=None):
-        interfaces.Interface.interactEOC(self, cycle)
-        if self._bocKeff is not None:
-            self.r.core.p.rxSwing = (
-                (self.r.core.p.keff - self._bocKeff)
-                / self._bocKeff
-                * units.ABS_REACTIVITY_TO_PCM
+            # track boc uncontrolled keff for rxSwing param.
+            self._bocKeff = self.r.core.p.keffUnc or self.r.core.p.keff
+
+        # A 1 burnstep cycle would have 2 nodes, and the last node would be node index 1 (first is zero)
+        lastNodeInCycle = getBurnSteps(self.cs)[self.r.p.cycle]
+        if self.r.p.timeNode == lastNodeInCycle and self._bocKeff is not None:
+
+            eocKeff = self.r.core.p.keffUnc or self.r.core.p.keff
+            swing = (eocKeff - self._bocKeff) / (eocKeff * self._bocKeff)
+            self.r.core.p.rxSwing = swing * units.ABS_REACTIVITY_TO_PCM
+            runLog.info(
+                f"BOC Uncontrolled keff: {self._bocKeff},  "
+                f"EOC Uncontrolled keff: {self.r.core.p.keffUnc}, "
+                f"Cycle Reactivity Swing: {self.r.core.p.rxSwing} pcm"
             )
 
     def checkEnergyBalance(self):
diff --git a/armi/reactor/parameters/parameterDefinitions.py b/armi/reactor/parameters/parameterDefinitions.py
index cafb3b26..a8a3c49d 100644
--- a/armi/reactor/parameters/parameterDefinitions.py
+++ b/armi/reactor/parameters/parameterDefinitions.py
@@ -32,7 +32,6 @@ import re
 
 import numpy
 
-from armi import runLog
 from armi.reactor.flags import Flags
 from armi.reactor.parameters.exceptions import ParameterError, ParameterDefinitionError
 
@@ -265,16 +264,12 @@ class Parameter:
         categories,
         serializer: Optional[Type[Serializer]] = None,
     ):
-        assert self._validName.match(name), "{} is not a valid param name".format(name)
         # nonsensical to have a serializer with no intention of saving to DB
         assert not (serializer is not None and not saveToDB)
         assert serializer is None or saveToDB
-        # TODO: This warning is temporary. At some point, it will become an AssertionError.
-        if not len(description):
-            runLog.warning(
-                f"DeprecationWarning: Parameter {name} defined without description.",
-                single=True,
-            )
+        assert self._validName.match(name), "{} is not a valid param name".format(name)
+        assert len(description), f"Parameter {name} defined without description."
+
         self.collectionType = _Undefined
         self.name = name
         self.fieldName = "_p_" + name
diff --git a/armi/settings/caseSettings.py b/armi/settings/caseSettings.py
index 8b76f72e..165359fd 100644
--- a/armi/settings/caseSettings.py
+++ b/armi/settings/caseSettings.py
@@ -467,7 +467,9 @@ class Settings:
                 elif key in settings.__settings:
                     settings.__settings[key].setValue(val)
                 else:
-                    settings.__settings[key] = Setting(key, val)
+                    settings.__settings[key] = Setting(
+                        key, val, description="Description from cs.modified()"
+                    )
 
         return settings
 
diff --git a/armi/settings/setting.py b/armi/settings/setting.py
index 286c2561..1d83cccc 100644
--- a/armi/settings/setting.py
+++ b/armi/settings/setting.py
@@ -65,7 +65,7 @@ class Setting:
         self,
         name,
         default,
-        description=None,
+        description,
         label=None,
         options=None,
         schema=None,
@@ -83,7 +83,7 @@ class Setting:
             the setting's name
         default : object
             The setting's default value
-        description : str, optional
+        description : str
             The description of the setting
         label : str, optional
             the shorter description used for the ARMI GUI
@@ -113,26 +113,23 @@ class Setting:
             will result in errors, requiring to user to update their input by hand to
             use more current settings.
         """
+        assert description, f"Setting {name} defined without description."
+        assert description != "None", f"Setting {name} defined without description."
+
         self.name = name
         self.description = description or name
-        if not description or description in ("None", "none"):
-            runLog.warning(
-                f"DeprecationWarning: Setting {name} defined without description.",
-                single=True,
-            )
         self.label = label or name
         self.options = options
         self.enforcedOptions = enforcedOptions
         self.subLabels = subLabels
         self.isEnvironment = isEnvironment
         self.oldNames: List[Tuple[str, Optional[datetime.date]]] = oldNames or []
-
         self._default = default
+        self._value = copy.deepcopy(default)  # break link from _default
         # Retain the passed schema so that we don't accidentally stomp on it in
         # addOptions(), et.al.
         self._customSchema = schema
         self._setSchema()
-        self._value = copy.deepcopy(default)  # break link from _default
 
     @property
     def underlyingType(self):
diff --git a/doc/release/0.3.rst b/doc/release/0.3.rst
index cfc31060..1371e01d 100644
--- a/doc/release/0.3.rst
+++ b/doc/release/0.3.rst
@@ -26,6 +26,7 @@ API Changes
 #. Removed unused methods: ``Reactor.getAllNuclidesIn()``, ``plotTriangleFlux()``. (`PR#1656 <https://github.com/terrapower/armi/pull/1656>`_)
 #. Removed ``armi.utils.dochelpers``; not relevant to nuclear modeling. (`PR#1662 <https://github.com/terrapower/armi/pull/1662>`_)
 #. Removing old tools created to help people convert to the current database format: ``armi.bookkeeping.db.convertDatabase()`` and ``ConvertDB``. (`PR#1658 <https://github.com/terrapower/armi/pull/1658>`_)
+#. Removing the unused method ``Case.buildCommand()``. (`PR#1773 <https://github.com/terrapower/armi/pull/1773>`_)
 #. Removed the variable ``armi.physics.neutronics.isotopicDepletion.ORDER``. (`PR#1671 <https://github.com/terrapower/armi/pull/1671>`_)
 #. Removing extraneous ``ArmiOjbect`` methods. (`PR#1667 <https://github.com/terrapower/armi/pull/1667>`_)
     * Moving ``ArmiObject.getBoronMassEnrich()`` to ``Block``.
@@ -47,7 +48,7 @@ API Changes
     * Moving ``ArmiObject.getTotalEnergyGenerationConstants`` to ``Block``.
     * Moving ``ArmiObject.getFissionEnergyGenerationConstants`` to ``Block``.
     * Moving ``ArmiObject.getCaptureEnergyGenerationConstants`` to ``Block``.
-#. Removing the parameeter ``rdIterNum``. (`PR#1704 <https://github.com/terrapower/armi/pull/1704>`_)
+#. Removing the parameter ``rdIterNum``. (`PR#1704 <https://github.com/terrapower/armi/pull/1704>`_)
 #. Removing the parameters ``outsideFuelRing`` and ``outsideFuelRingFluxFr``. (`PR#1700 <https://github.com/terrapower/armi/pull/1700>`_)
 #. Removing the setting ``doOrificedTH``. (`PR#1706 <https://github.com/terrapower/armi/pull/1706>`_)
 #. Changing the Doppler constant params to ``VOLUME_INTEGRATED``. (`PR#1659 <https://github.com/terrapower/armi/pull/1659>`_)
@@ -62,6 +63,7 @@ Bug Fixes
 #. Fixed ``safeCopy`` to work on both Windows and Linux with strict permissions (`PR#1691 <https://github.com/terrapower/armi/pull/1691>`_)
 #. When creating a new XS group, inherit settings from initial group. (`PR#1653 <https://github.com/terrapower/armi/pull/1653>`_, `PR#1751 <https://github.com/terrapower/armi/pull/1751>`_)
 #. Fixed a bug with ``Core.getReactionRates``. (`PR#1771 <https://github.com/terrapower/armi/pull/1771>`_)
+#. Fixed a bug with interactive versus batch mode checking on windows versus linux. (`PR#1786 <https://github.com/terrapower/armi/pull/1786>`_)
 #. TBD
 
 Quality Work
diff --git a/doc/tutorials/armi-example-app b/doc/tutorials/armi-example-app
index 60becb51..9279d8b9 160000
--- a/doc/tutorials/armi-example-app
+++ b/doc/tutorials/armi-example-app
@@ -1,1 +1,1 @@
-Subproject commit 60becb5137cf3c3671ebd44b06b894287611c181
+Subproject commit 9279d8b9f69ccfd66be5501944d1cf2f004daf6f


Test patch:
diff --git a/armi/cases/tests/test_cases.py b/armi/cases/tests/test_cases.py
index 095cb9b6..4c44689b 100644
--- a/armi/cases/tests/test_cases.py
+++ b/armi/cases/tests/test_cases.py
@@ -47,8 +47,7 @@ GEOM_INPUT = """<?xml version="1.0" ?>
     <assembly name="A3" pos="1"  ring="2"/>
 </reactor>
 """
-# This gets made into a StringIO multiple times because
-# it gets read multiple times.
+# This gets made into a StringIO multiple times because it gets read multiple times.
 
 BLUEPRINT_INPUT = """
 nuclide flags:
@@ -429,10 +428,6 @@ class TestCaseSuiteDependencies(unittest.TestCase):
         self.c1.title = "new_bob"
         self.assertEqual(self.c1.title, "new_bob")
 
-    def test_buildCommand(self):
-        cmd = self.c1.buildCommand()
-        self.assertEqual(cmd, 'python -u  -m armi run "c1.yaml"')
-
 
 class TestCaseSuiteComparison(unittest.TestCase):
     """CaseSuite.compare() tests."""
diff --git a/armi/physics/neutronics/globalFlux/tests/test_globalFluxInterface.py b/armi/physics/neutronics/globalFlux/tests/test_globalFluxInterface.py
index e0b61998..f2aed378 100644
--- a/armi/physics/neutronics/globalFlux/tests/test_globalFluxInterface.py
+++ b/armi/physics/neutronics/globalFlux/tests/test_globalFluxInterface.py
@@ -171,14 +171,31 @@ class TestGlobalFluxInterface(unittest.TestCase):
         Check that a 1000 pcm rx swing is observed due to the mock.
         """
         cs = settings.Settings()
+        cs["burnSteps"] = 2
         _o, r = test_reactors.loadTestReactor(
             inputFileName="smallestTestReactor/armiRunSmallest.yaml"
         )
         gfi = MockGlobalFluxInterface(r, cs)
+        bocKeff = 1.1
+        r.core.p.keffUnc = 1.1
         gfi.interactBOC()
+
+        r.p.cycle, r.p.timeNode = 0, 0
         gfi.interactEveryNode(0, 0)
-        gfi.interactEOC()
-        self.assertAlmostEqual(r.core.p.rxSwing, 1000)
+        self.assertAlmostEqual(gfi._bocKeff, r.core.p.keffUnc)
+        r.core.p.keffUnc = 1.05
+        r.p.cycle, r.p.timeNode = 0, 1
+        gfi.interactEveryNode(0, 1)
+        # doesn't change since its not the first node
+        self.assertAlmostEqual(gfi._bocKeff, bocKeff)
+        r.core.p.keffUnc = 1.01
+        r.p.cycle, r.p.timeNode = 0, 2
+        gfi.interactEveryNode(0, 2)
+        self.assertAlmostEqual(gfi._bocKeff, bocKeff)
+        self.assertAlmostEqual(r.core.p.rxSwing, -1e5 * (1.1 - 1.01) / (1.1 * 1.01))
+        gfi.interactBOC(0)
+        # now its zeroed at BOC
+        self.assertAlmostEqual(r.core.p.rxSwing, 0)
 
     def test_getIOFileNames(self):
         cs = settings.Settings()
diff --git a/armi/settings/tests/test_settings.py b/armi/settings/tests/test_settings.py
index 79c29f81..a82a0142 100644
--- a/armi/settings/tests/test_settings.py
+++ b/armi/settings/tests/test_settings.py
@@ -312,7 +312,7 @@ assemblyRotationAlgorithm: buReducingAssemblyRotatoin
             :id: T_ARMI_SETTINGS_DEFAULTS
             :tests: R_ARMI_SETTINGS_DEFAULTS
         """
-        a = setting.Setting("testsetting", 0)
+        a = setting.Setting("testsetting", 0, description="whatever")
         newDefault = setting.Default(5, "testsetting")
         a.changeDefault(newDefault)
         self.assertEqual(a.value, 5)
@@ -399,7 +399,7 @@ assemblyRotationAlgorithm: buReducingAssemblyRotatoin
         removed by Setting.__getstate__, and that has been a problem in the past.
         """
         # get a baseline: show how the Setting object looks to start
-        s1 = setting.Setting("testCopy", 765)
+        s1 = setting.Setting("testCopy", 765, description="whatever")
         self.assertEqual(s1.name, "testCopy")
         self.assertEqual(s1._value, 765)
         self.assertTrue(hasattr(s1, "schema"))
@@ -417,7 +417,7 @@ assemblyRotationAlgorithm: buReducingAssemblyRotatoin
         when the Setting value is set to a non-default value.
         """
         # get a baseline: show how the Setting object looks to start
-        s1 = setting.Setting("testCopy", 765)
+        s1 = setting.Setting("testCopy", 765, description="whatever")
         s1.value = 999
         self.assertEqual(s1.name, "testCopy")
         self.assertEqual(s1._value, 999)
@@ -487,7 +487,9 @@ class TestFlagListSetting(unittest.TestCase):
         flagsAsStringList = ["DUCT", "FUEL", "CLAD"]
         flagsAsFlagList = [Flags.DUCT, Flags.FUEL, Flags.CLAD]
 
-        fs = setting.FlagListSetting(name="testFlagSetting", default=[])
+        fs = setting.FlagListSetting(
+            name="testFlagSetting", default=[], description="whatever"
+        )
         # Set the value as a list of strings first
         fs.value = flagsAsStringList
         self.assertEqual(fs.value, flagsAsFlagList)
@@ -500,7 +502,9 @@ class TestFlagListSetting(unittest.TestCase):
 
     def test_invalidFlagListTypeError(self):
         """Test raising a TypeError when a list is not provided."""
-        fs = setting.FlagListSetting(name="testFlagSetting", default=[])
+        fs = setting.FlagListSetting(
+            name="testFlagSetting", default=[], description="whatever"
+        )
         with self.assertRaises(TypeError):
             fs.value = "DUCT"