Repository: terrapower/armi
Pull request #1178 (2023-02-10 23:31:39)

Cleanup Interface/Event `runLog.header`
## Description
Prior to this PR, the `Operator` would print out the wrong information for tightly coupled calculations. Rather then print the current iteration number, the operator would print "cycle" before printing the iteration number. 

E.g., within cycle 0 time node 0, the following would get printed to the stdout. 
```
===========  Triggering EveryNode - cycle 0, node 0 Event ===========
=========== 01 - main                           EveryNode - cycle 0, node 0 ===========
=========== 02 - fissionProducts                EveryNode - cycle 0, node 0 ===========
=========== 03 - xsGroups                       EveryNode - cycle 0, node 0 ===========
...
===========  Triggering Coupled - cycle 0 Event ===========
=========== 01 - main                           Coupled - cycle 0 ===========
=========== 02 - fissionProducts                Coupled - cycle 0 ===========
=========== 03 - xsGroups                       Coupled - cycle 0 ===========
... 
===========  Triggering Coupled - cycle 1 Event ===========
=========== 01 - main                           Coupled - cycle 1 ===========
=========== 02 - fissionProducts                Coupled - cycle 1 ===========
=========== 03 - xsGroups                       Coupled - cycle 1 ===========
```

This should rather read the following.
```
===========  Triggering EveryNode - cycle 0, node 0 Event ===========
=========== 01 - main                           EveryNode - cycle 0, node 0 ===========
...
===========  Triggering Coupled - iteration 0 Event ===========
=========== 01 - main                           Coupled - iteration 0 ===========
...
===========  Triggering Coupled - iteration 1 Event ===========
=========== 01 - main                           Coupled - iteration 1 ===========
```

Unit tests were also added to ensure that the correct string within `Operator::expandCycleAndTimeNodeArgs` gets printed for each event type and interface. 

<!-- Mandatory: Please include a summary of the change and link to any related GitHub Issues.-->

---

## 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 bug fixes or new features.
- [x] The documentation is still up-to-date in the `doc` folder.
- [x] The dependencies are still up-to-date in `setup.py`.



albeanth: @jakehader @onufer Just an FYI. 
keckler: I am a big fan, and would be an even bigger fan if it were even _more_ descriptive, like:
```
===========  Triggering EveryNode - cycle 0, node 0 Event ===========
=========== 01 - main                           EveryNode - cycle 0, node 0 ===========
...
===========  Triggering Coupled - iteration 0 Event ===========
=========== 01 - main                           Coupled - cycle 0, node 0, iteration 0 ===========
...
===========  Triggering Coupled - iteration 1 Event ===========
=========== 01 - main                           Coupled - cycle 0, node 0, iteration 1 ===========
```

I really hate having to scroll around to know where in the calculation the printout is referring.
albeanth: @keckler great idea! I originally had something like this in a git stash and had in my head that it would require a bigger API change. Turns out it didn't. And ended up resulting in a way less intrusive change! So double score. 
https://github.com/terrapower/armi/pull/1178/commits/d0d93e95eecabf8400a66cc85b74c91826b13019

Running one of our in-house tests now to make sure the stdout is right.  
albeanth: > @keckler great idea! I originally had something like this in a git stash and had in my head that it would require a bigger API change. Turns out it didn't. And ended up resulting in a way less intrusive change! So double score. [d0d93e9](https://github.com/terrapower/armi/commit/d0d93e95eecabf8400a66cc85b74c91826b13019)
> 
> Running one of our in-house tests now to make sure the stdout is right.

Jk. The in-house test just failed because the bigger API change is in fact needed. I would suggest that we revert back to https://github.com/terrapower/armi/pull/1178/commits/fcfbcf93d80e5f5178eabd1585f098eff1a14dc7 then address your suggestions post our big internal deliverable. I will require a change to `interactCoupled` methods on all of the interfaces. Less than ideal right now I think....

Thoughts? 
keckler: No big deal, happy for this small change anyway.

Patch:
diff --git a/armi/operators/operator.py b/armi/operators/operator.py
index 3a0b1716..c39b5669 100644
--- a/armi/operators/operator.py
+++ b/armi/operators/operator.py
@@ -424,7 +424,9 @@ class Operator:  # pylint: disable=too-many-public-methods
 
         halt = False
 
-        cycleNodeTag = self._expandCycleAndTimeNodeArgs(*args)
+        cycleNodeTag = self._expandCycleAndTimeNodeArgs(
+            *args, interactionName=interactionName
+        )
         runLog.header(
             "===========  Triggering {} Event ===========".format(
                 interactionName + cycleNodeTag
@@ -480,18 +482,34 @@ class Operator:  # pylint: disable=too-many-public-methods
         This looks better as multiple lines but it's a lot easier to grep as one line.
         We leverage newlines instead of long banners to save disk space.
         """
-        nodeInfo = self._expandCycleAndTimeNodeArgs(*args)
+        nodeInfo = self._expandCycleAndTimeNodeArgs(
+            *args, interactionName=interactionName
+        )
         line = "=========== {:02d} - {:30s} {:15s} ===========".format(
             statePointIndex, interface.name, interactionName + nodeInfo
         )
         runLog.header(line)
 
     @staticmethod
-    def _expandCycleAndTimeNodeArgs(*args):
-        """Return text annotating the (cycle, time node) args for each that are present."""
+    def _expandCycleAndTimeNodeArgs(*args, interactionName):
+        """Return text annotating information for current run event.
+
+        Notes
+        -----
+        - Init, BOL, EOL: empty
+        - Everynode: (cycle, time node)
+        - BOC: cycle number
+        - Coupling: iteration number
+        """
         cycleNodeInfo = ""
-        for label, step in zip((" - cycle {}", ", node {}"), args):
-            cycleNodeInfo += label.format(step)
+        if args:
+            if len(args) == 1:
+                if interactionName == "Coupled":
+                    cycleNodeInfo = f" - iteration {args[0]}"
+                elif interactionName in ("BOC", "EOC"):
+                    cycleNodeInfo = f" - cycle {args[0]}"
+            else:
+                cycleNodeInfo = f" - cycle {args[0]}, node {args[1]}"
         return cycleNodeInfo
 
     def _debugDB(self, interactionName, interfaceName, statePointIndex=0):
diff --git a/armi/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py
index 3c1dd256..e6d74ea0 100644
--- a/armi/settings/fwSettings/globalSettings.py
+++ b/armi/settings/fwSettings/globalSettings.py
@@ -100,7 +100,6 @@ CONF_ZONE_DEFINITIONS = "zoneDefinitions"
 CONF_ACCEPTABLE_BLOCK_AREA_ERROR = "acceptableBlockAreaError"
 CONF_FLUX_RECON = "fluxRecon"  # strange coupling in fuel handlers
 CONF_INDEPENDENT_VARIABLES = "independentVariables"
-CONF_HCF_CORETYPE = "HCFcoretype"
 CONF_T_IN = "Tin"
 CONF_T_OUT = "Tout"
 CONF_DEFERRED_INTERFACES_CYCLE = "deferredInterfacesCycle"
@@ -742,14 +741,6 @@ def defineSettings() -> List[setting.Setting]:
             description="List of (independentVarName, value) tuples to inform "
             "optimization post-processing",
         ),
-        setting.Setting(
-            CONF_HCF_CORETYPE,
-            default="TWRC",
-            label="Hot Channel Factor Set",
-            description="Switch to apply different sets of hot channel factors based "
-            "on design being analyzed",
-            options=["TWRC", "TWRP", "TWRC-HEX"],
-        ),
         setting.Setting(
             CONF_T_IN,
             default=360.0,
diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst
index 2d8570f1..ca451261 100644
--- a/doc/release/0.2.rst
+++ b/doc/release/0.2.rst
@@ -8,12 +8,12 @@ Release Date: TBD
 
 What's new in ARMI
 ------------------
-#. TBD
+#. Remove unused `HCFcoretype` setting. (`PR#1179 <https://github.com/terrapower/armi/pull/1179>`_)
 #. TBD
 
 Bug fixes
 ---------
-#. Fixed TBD
+#. Fixed interface/event ``runLog.header`` for tight coupling. (`PR#1178 https://github.com/terrapower/armi/pull/1178`_) 
 #. Fixed TBD
 
 


Test patch:
diff --git a/armi/operators/tests/test_inspectors.py b/armi/operators/tests/test_inspectors.py
index 0c3d73c9..9f0b0ae6 100644
--- a/armi/operators/tests/test_inspectors.py
+++ b/armi/operators/tests/test_inspectors.py
@@ -87,13 +87,7 @@ class TestInspector(unittest.TestCase):
 
     def test_assignCS(self):
         keys = sorted(self.inspector.cs.keys())
-        self.assertIn("HCFcoretype", keys)
-
-        self.assertEqual(self.inspector.cs["HCFcoretype"], "TWRC")
-        self.inspector._assignCS(
-            "HCFcoretype", "FAKE"
-        )  # pylint: disable=protected-access
-        self.assertEqual(self.inspector.cs["HCFcoretype"], "FAKE")
+        self.assertIn("nCycles", keys)
 
     def test_createQueryRevertBadPathToDefault(self):
         query = createQueryRevertBadPathToDefault(self.inspector, "numProcessors")
diff --git a/armi/operators/tests/test_operators.py b/armi/operators/tests/test_operators.py
index 61a99f2f..aa7a318b 100644
--- a/armi/operators/tests/test_operators.py
+++ b/armi/operators/tests/test_operators.py
@@ -286,5 +286,37 @@ settings:
         self.assertEqual(self.detailedOperator.maxBurnSteps, self.maxBurnStepsSolution)
 
 
+class TestInterfaceAndEventHeaders(unittest.TestCase):
+    def test_expandCycleAndTimeNodeArgs_Empty(self):
+        """when *args are empty, cycleNodeInfo should be an empty string"""
+        for task in ["Init", "BOL", "EOL"]:
+            self.assertEqual(
+                Operator._expandCycleAndTimeNodeArgs(interactionName=task), ""
+            )
+
+    def test_expandCycleAndTimeNodeArgs_OneArg(self):
+        """when *args is a single value, cycleNodeInfo should return the right string"""
+        cycle = 0
+        for task in ["BOC", "EOC"]:
+            self.assertEqual(
+                Operator._expandCycleAndTimeNodeArgs(cycle, interactionName=task),
+                f" - cycle {cycle}",
+            )
+        self.assertEqual(
+            Operator._expandCycleAndTimeNodeArgs(cycle, interactionName="Coupled"),
+            f" - iteration {cycle}",
+        )
+
+    def test_expandCycleAndTimeNodeArgs_TwoArg(self):
+        """when *args is two values, cycleNodeInfo should return the right string"""
+        cycle, timeNode = 0, 0
+        self.assertEqual(
+            Operator._expandCycleAndTimeNodeArgs(
+                cycle, timeNode, interactionName="EveryNode"
+            ),
+            f" - cycle {cycle}, node {timeNode}",
+        )
+
+
 if __name__ == "__main__":
     unittest.main()