From f59120fbaa30970c0ac95c25d79c289c11c3fae1 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Tue, 1 Aug 2023 12:21:48 -0700 Subject: [PATCH 01/17] Moving the First-Time Contributors guide to the docs (#1368) --- CONTRIBUTING.md | 78 +------------------ README.rst | 9 ++- .../parameters/parameterDefinitions.py | 3 +- doc/developer/first_time_contributors.rst | 76 ++++++++++++++++++ doc/developer/index.rst | 6 +- doc/developer/standards_and_practices.rst | 2 +- 6 files changed, 94 insertions(+), 80 deletions(-) create mode 100644 doc/developer/first_time_contributors.rst diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8ab8b9746..da7a88d95 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,77 +1,3 @@ -# How to contribute +# How to Contribute -The ARMI framework project strongly encourages developers to help contribute to the codebase. - -The ARMI framework code is open source, and your contributions will become open source. -Although fewer laws apply to open source materials because they are publicly-available, you still -must comply with all applicable laws and regulations. - -## Help Wanted - -There are a lot of things we need help with right off the bat, to get your feet wet: - -* Many more type annotations are desired. Type issues cause lots of bugs. -* Fewer Pylint warnings -* Better documentation -* Better test coverage -* Targeted speedups (e.g. informed by a profiler) -* Additional relevance to thermal reactors - -Naturally, we encourage other kinds of contributions as well. - -## Testing - -Any contribution must pass all included unit tests. The tests are built and run with the -`pytest` system. Please add new tests if you add new functionality. You can generally just run -`tox` to build the testing environment and execute all the tests and ruff checks. - -## Submitting changes - -Please send a [GitHub Pull Request to the ARMI main branch](https://github.com/terrapower/armi/pull/new/main) with a clear -list of what you've done (read more about [pull requests](http://help.github.com/pull-requests/)). Please follow our -coding conventions (below) and make sure all of your commits are atomic (one feature per commit). - -Please write a clear log messages for your commits. One-liners are OK for small changes, but bigger changes should include more: - - $ git commit -m "A brief summary of the commit - > - > A paragraph describing what changed and its impact." - -Note that a bot will require that you sign [our Contributor License -Agreement](https://gist.github.com/youngmit/8654abcf93f309771ae9296abebe9d4a) -before we can accept a pull request from you. - -## Coding conventions - -We use the [Black](https://black.readthedocs.io/en/stable/) code formatter so we don't have to argue or worry about trivial -whitespacing conventions during code review. You should always run `black` before submitting a pull request. - -We really like Robert C Martin's [Clean Code](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882) book -and at least nominally try to follow some of the advice in there. - - * Consider contacting some ARMI developers before embarking on a major new feature or development on the framework. - We may prefer to keep various physics or design-specific developments in modular plugins (which can also be - open source, of course!). The framework should be focused on frameworky stuff (broad, sharable stuff). - As the Framework matures, we hope it will stabilize, though we recognize we need work before then. - * We believe that comments can increase maintenance burdens and should only be used when one fails to explain - the entire situation with the code itself. In practice, we fail to do this all the time, and need comments - to help guide future readers of the code. - * Names //really// matter. - * Your PR will be reviewed and probably need some iteration. We aren't trying to be a pain or discourage you, - we just want to make sure the code is optimal. - -## Licensing of Tools - -Be careful when including any dependency in ARMI (say in a requirements.txt file) not -to include anything with a license that superceeds our Apache license. For instance, -any third-party Python library included in ARMI with a GPL license will make the whole -project fall under the GPL license. But a lot of potential users of ARMI will want to -keep some of their work private, so we can't allow any GPL tools. - -For that reason, it is generally considered best-practice in the ARMI ecosystem to -only use third-party Python libraries that have MIT or BSD licenses. - -## Documentation - -We use Sphinx for our documentation, and numpydoc to parse docstrings into the API docs section of our documentation. -Thus, all docstrings are officially part of the technical documentation and should be written as such. +For information on how to contribute to ARMI, see [our official documenation](https://terrapower.github.io/armi/developer/first_time_contributors.html). diff --git a/README.rst b/README.rst index 7ffe27103..f44420b5c 100644 --- a/README.rst +++ b/README.rst @@ -49,6 +49,8 @@ found in [#touranarmi]_. - https://github.com/terrapower/armi * - Documentation - https://terrapower.github.io/armi + * - First time contributor's guide + - https://terrapower.github.io/armi/developer/first_time_contributors.html * - Bug tracker - https://github.com/terrapower/armi/issues * - Plugin directory @@ -58,7 +60,7 @@ found in [#touranarmi]_. Quick start ----------- -Before starting, you need to have `Python `_ 3.7+ on +Before starting, you need to have `Python `_ 3.9+ on Windows or Linux. Get the ARMI code, install the prerequisites, and fire up the launcher with the following @@ -83,6 +85,11 @@ and then run:: This runs the unit tests in parallel on 6 processes. Omit the ``-n 6`` argument to run on a single process. +The tests can also be run directly, using ``pytest``:: + + $ pip3 install -r requirements-testing.txt + $ pytest -n 4 armi + From here, we recommend going through a few of our `gallery examples `_ and `tutorials `_ to diff --git a/armi/reactor/parameters/parameterDefinitions.py b/armi/reactor/parameters/parameterDefinitions.py index 13ead3307..e1163ebde 100644 --- a/armi/reactor/parameters/parameterDefinitions.py +++ b/armi/reactor/parameters/parameterDefinitions.py @@ -254,7 +254,8 @@ def __init__( # 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." + f"DeprecationWarning: Parameter {name} defined without description.", + single=True, ) self.collectionType = _Undefined self.name = name diff --git a/doc/developer/first_time_contributors.rst b/doc/developer/first_time_contributors.rst new file mode 100644 index 000000000..93a0d64ae --- /dev/null +++ b/doc/developer/first_time_contributors.rst @@ -0,0 +1,76 @@ +***************************** +First Time Contributors Guide +***************************** + +The ARMI team strongly encourages developers to contribute to the codebase. + +The ARMI framework code is open source, and your contributions will become open source. +Although fewer laws apply to open source materials because they are publicly-available, you still +must comply with all applicable laws and regulations. + +Help Wanted +=========== + +There are a lot of places you can get started to help the ARMI project and team: + +* Better :doc:`documentation ` +* Better test coverage +* Many more type annotations are desired. Type issues cause lots of bugs. +* Targeted speedups (e.g. informed by a profiler) +* Additional relevance to thermal reactors + +Naturally, you can also look at the open `ARMI issues `_ to see what work needs to be done. In particular, check out the `help wanted tickets `_ and `good first issue tickets `_. + +Testing +======= + +Any contribution must pass all included unit tests. You will frequently have to fix +tests your code changes break. And you should definitely add tests to cover anything +new your code does. + +The standard way to run the tests is to install and use `tox `_:: + + $ pip3 install tox + $ tox -- -n 6 + +This runs the unit tests in parallel on 6 processes. Omit the ``-n 6`` argument +to run on a single process. + +Or the tests can also be run using ``pytest`` directly:: + + $ pip3 install -r requirements-testing.txt + $ pytest -n 4 armi + +Submitting Changes +================== + +To submit a change to ARMI, you will have to open a Pull Request (PR) on GitHub.com. + +The process for opening a PR against ARMI goes something like this: + +1. `Fork the ARMI repo `_ +2. `Create a new branch `_ in your repo +3. Make your code changes to your new branch +4. Submit a Pull Request against `ARMIs main branch `_ + a. See `GitHubs general guidance on Pull Requests `_ + b. See :doc:`ARMIs specific guidance ` on what makes a "good" Pull Request. +5. Actively engage with your PR reviewer's questions and comments. + +> Note that a bot will require that you sign our `Contributor License Agreement `_ +before we can accept a pull request from you. + +See our published documentation for a complete guide to our :doc:`coding standards and practices `. + +Also, please check out our (quick) synopsis on :doc:`good commit messages `. + +Licensing of Tools +================== + +Be careful when including any dependency in ARMI (say in a ``requirements.txt`` file) not +to include anything with a license that superceeds our Apache license. For instance, +any third-party Python library included in ARMI with a GPL license will make the whole +project fall under the GPL license. But a lot of potential users of ARMI will want to +keep some of their work private, so we can't allow any GPL tools. + +For that reason, it is generally considered best-practice in the ARMI ecosystem to +only use third-party Python libraries that have MIT or BSD licenses. diff --git a/doc/developer/index.rst b/doc/developer/index.rst index cc9879f2a..23e426759 100644 --- a/doc/developer/index.rst +++ b/doc/developer/index.rst @@ -20,4 +20,8 @@ ARMI for the community. guide making_armi_based_apps entrypoints - * + documenting + parallel_coding + profiling + reports + * \ No newline at end of file diff --git a/doc/developer/standards_and_practices.rst b/doc/developer/standards_and_practices.rst index a8f239871..3d7464a29 100644 --- a/doc/developer/standards_and_practices.rst +++ b/doc/developer/standards_and_practices.rst @@ -186,7 +186,7 @@ code, consider pulling the repeated code out into it's own function, or using a Public methods should have docstrings ===================================== -Always create the :doc:`proper docstrings ` for all public +Always create the `proper docstrings `_ for all public functions and public classes. Unit tests From e8ddcdf774f9eb3e842ae75948f095095fc2d48b Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Tue, 1 Aug 2023 15:19:44 -0700 Subject: [PATCH 02/17] Removing global variable from runLog.py (#1370) There is no reason the "white space" value need be global. I just put it into a staticmethod. --- armi/runLog.py | 57 +++++++++++++++++++++++++++------------ armi/tests/test_runLog.py | 13 +++++++++ 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/armi/runLog.py b/armi/runLog.py index b87e47b72..b69ecca88 100644 --- a/armi/runLog.py +++ b/armi/runLog.py @@ -61,7 +61,6 @@ if self.isEnabledFor({1}): self._log({1}, message, args, **kws) logging.Logger.{0} = {0}""" -_WHITE_SPACE = " " * 6 LOG_DIR = os.path.join(os.getcwd(), "logs") OS_SECONDS_TIMEOUT = 2 * 60 SEP = "|" @@ -88,7 +87,6 @@ def __init__(self, mpiRank=0): ---------- mpiRank : int If this is zero, we are in the parent process, otherwise child process. - The default of 0 means we assume the parent process. This should not be adjusted after instantiation. """ self._mpiRank = mpiRank @@ -107,25 +105,49 @@ def setNullLoggers(self): self.logger = NullLogger("NULL") self.stderrLogger = NullLogger("NULL2", isStderr=True) - def _setLogLevels(self): - """Here we fill the logLevels dict with custom strings that depend on the MPI rank.""" + @staticmethod + def getLogLevels(mpiRank): + """Helper method to build an important data object this class needs. + + Parameters + ---------- + mpiRank : int + If this is zero, we are in the parent process, otherwise child process. + This should not be adjusted after instantiation. + """ + rank = "" if mpiRank == 0 else "-{:>03d}".format(mpiRank) + # NOTE: using ordereddict so we can get right order of options in GUI - _rank = "" if self._mpiRank == 0 else "-{:>03d}".format(self._mpiRank) - self.logLevels = collections.OrderedDict( + return collections.OrderedDict( [ - ("debug", (logging.DEBUG, "[dbug{}] ".format(_rank))), - ("extra", (15, "[xtra{}] ".format(_rank))), - ("info", (logging.INFO, "[info{}] ".format(_rank))), - ("important", (25, "[impt{}] ".format(_rank))), - ("prompt", (27, "[prmt{}] ".format(_rank))), - ("warning", (logging.WARNING, "[warn{}] ".format(_rank))), - ("error", (logging.ERROR, "[err {}] ".format(_rank))), - ("header", (100, "{}".format(_rank))), + ("debug", (logging.DEBUG, f"[dbug{rank}] ")), + ("extra", (15, f"[xtra{rank}] ")), + ("info", (logging.INFO, f"[info{rank}] ")), + ("important", (25, f"[impt{rank}] ")), + ("prompt", (27, f"[prmt{rank}] ")), + ("warning", (logging.WARNING, f"[warn{rank}] ")), + ("error", (logging.ERROR, f"[err {rank}] ")), + ("header", (100, f"{rank}")), ] ) + + @staticmethod + def getWhiteSpace(mpiRank): + """Helper method to build the white space used to left-adjust the log lines. + + Parameters + ---------- + mpiRank : int + If this is zero, we are in the parent process, otherwise child process. + This should not be adjusted after instantiation. + """ + logLevels = _RunLog.getLogLevels(mpiRank) + return " " * len(max([ll[1] for ll in logLevels.values()])) + + def _setLogLevels(self): + """Here we fill the logLevels dict with custom strings that depend on the MPI rank.""" + self.logLevels = self.getLogLevels(self._mpiRank) self._logLevelNumbers = sorted([ll[0] for ll in self.logLevels.values()]) - global _WHITE_SPACE - _WHITE_SPACE = " " * len(max([ll[1] for ll in self.logLevels.values()])) # modify the logging module strings for printing for longLogString, (logValue, shortLogString) in self.logLevels.items(): @@ -462,7 +484,8 @@ def filter(self, record): return False # Handle some special string-mangling we want to do, for multi-line messages - record.msg = msg.rstrip().replace("\n", "\n" + _WHITE_SPACE) + whiteSpace = _RunLog.getWhiteSpace(context.MPI_RANK) + record.msg = msg.rstrip().replace("\n", "\n" + whiteSpace) return True diff --git a/armi/tests/test_runLog.py b/armi/tests/test_runLog.py index fb70f8f0f..4df8d8871 100644 --- a/armi/tests/test_runLog.py +++ b/armi/tests/test_runLog.py @@ -88,6 +88,15 @@ def test_parentRunLogging(self): self.assertIn("Hello", streamVal, msg=streamVal) self.assertIn("world", streamVal, msg=streamVal) + def test_getWhiteSpace(self): + log = runLog._RunLog(0) + space0 = len(log.getWhiteSpace(0)) + space1 = len(log.getWhiteSpace(1)) + space9 = len(log.getWhiteSpace(9)) + + self.assertGreater(space1, space0) + self.assertEqual(space1, space9) + def test_warningReport(self): """A simple test of the warning tracking and reporting logic.""" # create the logger and do some logging @@ -125,6 +134,10 @@ def test_warningReport(self): self.assertNotIn("invisible", streamVal, msg=streamVal) self.assertEqual(streamVal.count("test_warningReport"), 2, msg=streamVal) + # bonus check: edge case in duplicates filter + log.logger = None + self.assertIsNone(log.getDuplicatesFilter()) + def test_warningReportInvalid(self): """A test of warningReport in an invalid situation.""" # create the logger and do some logging From 4654b9649cf39392ac0db941f0a561c97dea6a58 Mon Sep 17 00:00:00 2001 From: Arrielle Opotowsky Date: Mon, 7 Aug 2023 19:30:30 -0500 Subject: [PATCH 03/17] Add Python 3.11 to GH Actions (#1341) --- .github/workflows/black.yaml | 4 ++-- .github/workflows/unittests.yaml | 2 +- .github/workflows/validatemanifest.yaml | 2 +- .github/workflows/wintests.yaml | 2 +- doc/release/0.2.rst | 1 + doc/requirements-docs.txt | 5 +++-- requirements-testing.txt | 3 --- setup.py | 7 ++++--- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/black.yaml b/.github/workflows/black.yaml index 513b2fb8f..4cad8b267 100644 --- a/.github/workflows/black.yaml +++ b/.github/workflows/black.yaml @@ -8,10 +8,10 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v2 - - name: Set up Python 3.10 + - name: Set up Python 3.11 uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.11' - name: Install Black run: pip install 'black==20.8b1' 'click==8.0.1' - name: Run black --check . diff --git a/.github/workflows/unittests.yaml b/.github/workflows/unittests.yaml index 0cde1b325..475d78017 100644 --- a/.github/workflows/unittests.yaml +++ b/.github/workflows/unittests.yaml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-22.04 strategy: matrix: - python: [3.7, 3.8, 3.9, '3.10'] + python: [3.7, 3.8, 3.9, '3.10', '3.11'] steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/validatemanifest.yaml b/.github/workflows/validatemanifest.yaml index c9504a0df..c9e86c371 100644 --- a/.github/workflows/validatemanifest.yaml +++ b/.github/workflows/validatemanifest.yaml @@ -12,7 +12,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.11' - name: Install Tox and any other packages run: pip install tox - name: Run Tox diff --git a/.github/workflows/wintests.yaml b/.github/workflows/wintests.yaml index 966118c87..4d1dd435d 100644 --- a/.github/workflows/wintests.yaml +++ b/.github/workflows/wintests.yaml @@ -12,7 +12,7 @@ jobs: - name: Setup Python uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.11' - name: Upgrade PIP run: python -m pip install --upgrade pip - name: Install deps diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst index f796a64ab..545ad762c 100644 --- a/doc/release/0.2.rst +++ b/doc/release/0.2.rst @@ -12,6 +12,7 @@ What's new in ARMI #. Broad cleanup of ``Parameters``: filled in all empty units and descriptions, removed unused params. (`PR#1345 `_) #. Removed redundant ``Material.name`` variable. (`PR#1335 `_) #. Added SHA1 hashes of XS control files to the welcome text. (`PR#1334 `_) +#. Add python 3.11 to ARMI's CI testing GH actions! (`PR#1341 `_) #. Put back ``avgFuelTemp`` block parameter. (`PR#1362 `_) #. Make cylindrical component block collection less strict about pre-homogenization checks. (`PR#1347 `_) #. TBD diff --git a/doc/requirements-docs.txt b/doc/requirements-docs.txt index 49d66466e..09f83334c 100644 --- a/doc/requirements-docs.txt +++ b/doc/requirements-docs.txt @@ -30,8 +30,9 @@ pandoc # iPython kernel to run Jupyter notebooks ipykernel==6.7.0 -# helps us use RST files -docutils<0.18 +# docutils 0.17 introduced a bug that prevents bullets from rendering +# see https://github.com/terrapower/armi/issues/274 +docutils <0.17 # used to generate UML diagrams pylint==2.7.4 diff --git a/requirements-testing.txt b/requirements-testing.txt index 1efa2670a..a30ad27ce 100644 --- a/requirements-testing.txt +++ b/requirements-testing.txt @@ -1,7 +1,4 @@ numpy>=1.21,<=1.23.5 ipython>=7.34.0,<=8.12.0 -# docutils 0.17 introduced a bug that prevents bullets from rendering -# see https://github.com/terrapower/armi/issues/274 -docutils <0.17 -e .[memprof,dev] diff --git a/setup.py b/setup.py index c105c6ad9..f253cd84b 100644 --- a/setup.py +++ b/setup.py @@ -56,7 +56,8 @@ def collectExtraFiles(): entry_points={"console_scripts": ["armi = armi.__main__:main"]}, install_requires=[ "configparser", - "coverage<=6.5.0", + 'coverage; python_version>="3.11"', + 'coverage<=6.5.0; python_version<"3.11"', "future", "h5py>=3.0", "htmltree", @@ -76,8 +77,8 @@ def collectExtraFiles(): "yamlize==0.7.1", ], extras_require={ - "mpi": ["mpi4py==3.0.3"], - "grids": ["wxpython<=4.1.1"], + "mpi": ["mpi4py"], + "grids": ["wxpython==4.2.1"], "memprof": ["psutil"], "dev": [ "black==20.8b1", From 06bf1f1806dda8ea63ed832282693df3b768a37d Mon Sep 17 00:00:00 2001 From: Nick Touran Date: Wed, 9 Aug 2023 10:47:34 -0700 Subject: [PATCH 04/17] Reconnect logger after disconnecting in test. (#1381) --- armi/tests/test_runLog.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/armi/tests/test_runLog.py b/armi/tests/test_runLog.py index 4df8d8871..15e5c1011 100644 --- a/armi/tests/test_runLog.py +++ b/armi/tests/test_runLog.py @@ -135,8 +135,9 @@ def test_warningReport(self): self.assertEqual(streamVal.count("test_warningReport"), 2, msg=streamVal) # bonus check: edge case in duplicates filter - log.logger = None + backupLog, log.logger = log.logger, None self.assertIsNone(log.getDuplicatesFilter()) + log.logger = backupLog def test_warningReportInvalid(self): """A test of warningReport in an invalid situation.""" From 18f3200914155c86851a635466cf86adaa4da063 Mon Sep 17 00:00:00 2001 From: Nick Touran Date: Wed, 9 Aug 2023 14:40:51 -0700 Subject: [PATCH 05/17] Remove coveragepy-lcov dependency during CI. (#1382) * Remove coveragepy-lcov dependency during CI. coveragepy-lcov is incompatible with coverage 7.0.0 and in fact is totally unnecessary now that pytest can write lcov format files directly. This switches pytest to write lcov directly. Now coverage should work in CI on python 3.11 Fixes #1377 * Remove coverage pins and change coverage to py311. Requested in review. --- .github/workflows/coverage.yaml | 16 ++-------------- .gitignore | 1 + setup.py | 3 +-- tox.ini | 4 ++-- 4 files changed, 6 insertions(+), 18 deletions(-) diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml index fd049661b..0e85a6400 100644 --- a/.github/workflows/coverage.yaml +++ b/.github/workflows/coverage.yaml @@ -8,19 +8,16 @@ on: jobs: build: - runs-on: ubuntu-22.04 - env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }} - steps: - uses: actions/checkout@v2 - name: Setup Python uses: actions/setup-python@v2 with: - python-version: '3.10' + python-version: '3.11' - name: Update package index run: sudo apt-get update - name: Install mpi libs @@ -31,17 +28,8 @@ jobs: run: tox -e cov1 || true - name: Run Coverage Part 2 run: tox -e cov2 - - name: Convert Coverage Results - env: - COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }} - run: | - pip install click==8.1.3 - pip install coverage==6.5.0 - pip install coveragepy-lcov==0.1.2 - coveragepy-lcov --data_file_path coverage_results.cov --output_file_path lcov.txt - name: Publish to coveralls.io uses: coverallsapp/github-action@v1.1.2 with: github-token: ${{ secrets.GITHUB_TOKEN }} - path-to-lcov: lcov.txt - + path-to-lcov: coverage.lcov diff --git a/.gitignore b/.gitignore index 9b209fa96..83bb002f7 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ doc/gallery doc/gallery-src/framework/*.yaml .coverage coverage.xml +coverage.lcov coverage_results.* htmlcov/ monkeytype.* diff --git a/setup.py b/setup.py index f253cd84b..9b5468d70 100644 --- a/setup.py +++ b/setup.py @@ -56,8 +56,7 @@ def collectExtraFiles(): entry_points={"console_scripts": ["armi = armi.__main__:main"]}, install_requires=[ "configparser", - 'coverage; python_version>="3.11"', - 'coverage<=6.5.0; python_version<"3.11"', + "coverage", "future", "h5py>=3.0", "htmltree", diff --git a/tox.ini b/tox.ini index 33eb32173..8795e5a92 100644 --- a/tox.ini +++ b/tox.ini @@ -39,7 +39,7 @@ deps= allowlist_externals = /usr/bin/mpiexec commands = - mpiexec -n 2 --use-hwthread-cpus coverage run --rcfile=.coveragerc -m pytest --cov=armi --cov-config=.coveragerc --ignore=venv --cov-fail-under=80 armi/tests/test_mpiFeatures.py + mpiexec -n 2 --use-hwthread-cpus coverage run --rcfile=.coveragerc -m pytest --cov=armi --cov-config=.coveragerc --cov-report=lcov --ignore=venv --cov-fail-under=80 armi/tests/test_mpiFeatures.py # Second, run code coverage over the rest of the unit tests, and combine the coverage results together [testenv:cov2] @@ -51,7 +51,7 @@ deps= allowlist_externals = /usr/bin/mpiexec commands = - coverage run --rcfile=.coveragerc -m pytest -n 4 --cov=armi --cov-config=.coveragerc --cov-append --ignore=venv armi + coverage run --rcfile=.coveragerc -m pytest -n 4 --cov=armi --cov-config=.coveragerc --cov-report=lcov --cov-append --ignore=venv armi coverage combine --rcfile=.coveragerc --keep -a # NOTE: This only runs the MPI unit tests. From b61e643e2cbdefa4a8dbab11009e4a39ac66b419 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Wed, 9 Aug 2023 16:38:53 -0700 Subject: [PATCH 06/17] Adding a little code coverage to the CLIs (#1371) --- armi/cli/tests/test_runEntryPoint.py | 42 ++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/armi/cli/tests/test_runEntryPoint.py b/armi/cli/tests/test_runEntryPoint.py index 7a09dd168..0838bde66 100644 --- a/armi/cli/tests/test_runEntryPoint.py +++ b/armi/cli/tests/test_runEntryPoint.py @@ -224,11 +224,17 @@ class TestExtractInputs(unittest.TestCase): def test_extractInputsBasics(self): ei = ExtractInputs() ei.addOptions() - ei.parse_args(["/path/to/fake.h5"]) + ei.parse_args(["/path/to/fake"]) self.assertEqual(ei.name, "extract-inputs") self.assertEqual(ei.args.output_base, "/path/to/fake") + with mockRunLogs.BufferLog() as mock: + self.assertEqual("", mock.getStdout()) + with self.assertRaises(FileNotFoundError): + # The "fake" file doesn't exist, so this should fail. + ei.invoke() + class TestInjectInputs(unittest.TestCase): def test_injectInputsBasics(self): @@ -239,6 +245,32 @@ def test_injectInputsBasics(self): self.assertEqual(ii.name, "inject-inputs") self.assertIsNone(ii.args.blueprints) + def test_injectInputsInvokeIgnore(self): + ii = InjectInputs() + ii.addOptions() + ii.parse_args(["/path/to/fake.h5"]) + + with mockRunLogs.BufferLog() as mock: + self.assertEqual("", mock.getStdout()) + ii.invoke() + self.assertIn("No settings", mock.getStdout()) + + def test_injectInputsInvokeNoData(self): + with TemporaryDirectoryChanger(): + # init CLI + ii = InjectInputs() + ii.addOptions() + + bp = os.path.join(TEST_ROOT, "refSmallReactor.yaml") + ii.parse_args(["/path/to/fake.h5", "--blueprints", bp]) + + # invoke and check log + with mockRunLogs.BufferLog() as mock: + self.assertEqual("", mock.getStdout()) + with self.assertRaises(FileNotFoundError): + # The "fake.h5" doesn't exist, so this should fail. + ii.invoke() + class TestMigrateInputs(unittest.TestCase): def test_migrateInputsBasics(self): @@ -287,11 +319,17 @@ class TestReportsEntryPoint(unittest.TestCase): def test_reportsEntryPointBasics(self): rep = ReportsEntryPoint() rep.addOptions() - rep.parse_args(["-h5db", "/path/to/fake.yaml"]) + rep.parse_args(["-h5db", "/path/to/fake.h5"]) self.assertEqual(rep.name, "report") self.assertEqual(rep.settingsArgument, "optional") + with mockRunLogs.BufferLog() as mock: + self.assertEqual("", mock.getStdout()) + with self.assertRaises(ValueError): + # The "fake.h5" doesn't exist, so this should fail. + rep.invoke() + class TestCompareIsotxsLibsEntryPoint(unittest.TestCase): def test_compareIsotxsLibsBasics(self): From 0846a86457c6a4b6715a19256628cd86c9603f92 Mon Sep 17 00:00:00 2001 From: Nick Touran Date: Wed, 9 Aug 2023 16:53:16 -0700 Subject: [PATCH 07/17] Updating documentation dependencies for Python 3.11 (#1378) * Update documentation reqs. This makes them compatible with Python 3.11 and 3.9. I confirmed with fresh venvs of both versions on Linux at least. * Fix CSS with latest RTD theme. This uses the modern recommended approach from the RTD theme docs for overriding a few css things using @import. Also bring in a few other theme options just for visibility. * Add underlines to all gallery examples. This is required for sphinx-gallery to build the galleries correctly. Otherwise you get sphinx build errors and rendering errors where the gallery examples cannot be clicked. * Pin docutils. This one is pulled between sphinx-rtd-theme and sphinx-needs. * Re-activate sphinx needs and fix one dependency. This was a mistake on previous commit. For On Python 3.9, building the docs caused a jinja error during nbconvert that requires pinning back to Jinja2==3.0.3 so that was also done. See: https://github.com/sphinx-doc/sphinx/issues/10289 --- doc/.static/{ => css}/theme_fixes.css | 2 + doc/conf.py | 24 +++++++---- .../analysis/run_blockMcnpMaterialCard.py | 3 +- .../framework/run_blockVolumeFractions.py | 3 +- .../framework/run_chartOfNuclides.py | 1 + .../framework/run_computeReactionRates.py | 1 + .../framework/run_fuelManagement.py | 1 + doc/gallery-src/framework/run_grids1_hex.py | 1 + .../framework/run_grids2_cartesian.py | 1 + doc/gallery-src/framework/run_grids3_rzt.py | 1 + doc/gallery-src/framework/run_isotxs.py | 1 + .../framework/run_isotxs2_matrix.py | 1 + doc/gallery-src/framework/run_materials.py | 1 + .../run_programmaticReactorDefinition.py | 1 + .../framework/run_reactorFacemap.py | 1 + .../framework/run_transmutationMatrix.py | 1 + doc/requirements-docs.txt | 43 +++++++++++-------- doc/requirements/srsd.rst | 2 +- 18 files changed, 59 insertions(+), 30 deletions(-) rename doc/.static/{ => css}/theme_fixes.css (91%) diff --git a/doc/.static/theme_fixes.css b/doc/.static/css/theme_fixes.css similarity index 91% rename from doc/.static/theme_fixes.css rename to doc/.static/css/theme_fixes.css index 976840f1e..e5d239b61 100644 --- a/doc/.static/theme_fixes.css +++ b/doc/.static/css/theme_fixes.css @@ -1,3 +1,5 @@ +@import 'theme.css'; + /* override table width restrictions */ @media screen and (min-width: 767px) { diff --git a/doc/conf.py b/doc/conf.py index eab91134c..3d0624215 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -137,7 +137,9 @@ def setup(app): "sphinx_gallery.gen_gallery", "sphinx.ext.imgconverter", # to convert GH Actions badge SVGs to PNG for LaTeX "sphinxcontrib.plantuml", - "sphinxcontrib.needs", + "sphinx_needs", + "sphinx_rtd_theme", # needed here for loading jquery in sphinx 6 + "sphinxcontrib.jquery", # see https://github.com/readthedocs/sphinx_rtd_theme/issues/1452 ] # Our API should make sense without documenting private/special members. @@ -238,11 +240,24 @@ def setup(app): html_theme_options = { "style_external_links": True, "style_nav_header_background": "#233C5B", # TP blue looks better than green + "logo_only": False, + "display_version": True, + "prev_next_buttons_location": "bottom", + "vcs_pageview_mode": "", + # Toc options + "collapse_navigation": True, + "sticky_navigation": True, + "navigation_depth": 4, + "includehidden": True, + "titles_only": False, } # Add any paths that contain custom themes here, relative to this directory. html_theme_path = [sphinx_rtd_theme.get_html_theme_path()] +# as long as this file @import's the theme's main css it won't break anything +html_style = "css/theme_fixes.css" + # The name of an image file (within the static path) to use as favicon of the # docs. This file should be a Windows icon file (.ico) being 16x16 or 32x32 # pixels large. @@ -260,14 +275,7 @@ def setup(app): # Output file base name for HTML help builder. htmlhelp_basename = "ARMIdoc" -# Need to manually add gallery css files or else the theme_fixes override them. html_context = { - "css_files": [ - "_static/theme_fixes.css", # overrides for wide tables in RTD theme - "_static/gallery.css", # for the sphinx-gallery plugin - "_static/gallery-binder.css", - "_static/gallery-dataframe.css", - ], "display_github": True, # Integrate GitHub "github_user": "terrapower", # Username "github_repo": "armi", # Repo name diff --git a/doc/gallery-src/analysis/run_blockMcnpMaterialCard.py b/doc/gallery-src/analysis/run_blockMcnpMaterialCard.py index f0da2649d..683a9c749 100644 --- a/doc/gallery-src/analysis/run_blockMcnpMaterialCard.py +++ b/doc/gallery-src/analysis/run_blockMcnpMaterialCard.py @@ -12,7 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. """ -Write MCNP Material Cards. +Write MCNP Material Cards +========================= Here we load a test reactor and write each component of one fuel block out as MCNP material cards. diff --git a/doc/gallery-src/framework/run_blockVolumeFractions.py b/doc/gallery-src/framework/run_blockVolumeFractions.py index 4ad090620..993cb6ba0 100644 --- a/doc/gallery-src/framework/run_blockVolumeFractions.py +++ b/doc/gallery-src/framework/run_blockVolumeFractions.py @@ -14,7 +14,8 @@ # -*- coding: utf-8 -*- """ -Computing Component Volume Fractions on a Block with Automatic Thermal Expansion. +Computing Component Volume Fractions on a Block with Automatic Thermal Expansion +================================================================================ Given an :py:mod:`Block `, compute the component volume fractions. Assess the change in volume diff --git a/doc/gallery-src/framework/run_chartOfNuclides.py b/doc/gallery-src/framework/run_chartOfNuclides.py index 4c6599c9b..13fcfe3ad 100644 --- a/doc/gallery-src/framework/run_chartOfNuclides.py +++ b/doc/gallery-src/framework/run_chartOfNuclides.py @@ -13,6 +13,7 @@ # limitations under the License. """ Plot a chart of the nuclides. +============================= Use the nuclide directory of ARMI to plot a chart of the nuclides coloring the squares with the natural abundance. diff --git a/doc/gallery-src/framework/run_computeReactionRates.py b/doc/gallery-src/framework/run_computeReactionRates.py index 585ae42e4..8639cdb71 100644 --- a/doc/gallery-src/framework/run_computeReactionRates.py +++ b/doc/gallery-src/framework/run_computeReactionRates.py @@ -13,6 +13,7 @@ # limitations under the License. """ Computing Reaction Rates on a Block. +==================================== In this example, a set of 1-group reaction rates (in #/s) are evaluated for a dummy fuel block containing UZr fuel, HT9 structure, and diff --git a/doc/gallery-src/framework/run_fuelManagement.py b/doc/gallery-src/framework/run_fuelManagement.py index a013bee8a..49895c093 100644 --- a/doc/gallery-src/framework/run_fuelManagement.py +++ b/doc/gallery-src/framework/run_fuelManagement.py @@ -13,6 +13,7 @@ # limitations under the License. """ Fuel management in a LWR. +========================= Demo of locating and swapping assemblies in a core with Cartesian geometry. Given a burnup distribution, this swaps high burnup assemblies with low ones. diff --git a/doc/gallery-src/framework/run_grids1_hex.py b/doc/gallery-src/framework/run_grids1_hex.py index 41733309f..7d39ca1f0 100644 --- a/doc/gallery-src/framework/run_grids1_hex.py +++ b/doc/gallery-src/framework/run_grids1_hex.py @@ -13,6 +13,7 @@ # limitations under the License. """ Make a hex grid. +================ This uses a grid factory method to build an infinite 2-D grid of hexagons with pitch equal to 1.0 cm. diff --git a/doc/gallery-src/framework/run_grids2_cartesian.py b/doc/gallery-src/framework/run_grids2_cartesian.py index 67d68e3e8..ac842b0af 100644 --- a/doc/gallery-src/framework/run_grids2_cartesian.py +++ b/doc/gallery-src/framework/run_grids2_cartesian.py @@ -13,6 +13,7 @@ # limitations under the License. """ Make a Cartesian grid. +====================== This builds a Cartesian grid with squares 1 cm square, with the z-coordinates provided explicitly. It is also offset in 3D space to X, Y, Z = 10, 5, 5 cm. diff --git a/doc/gallery-src/framework/run_grids3_rzt.py b/doc/gallery-src/framework/run_grids3_rzt.py index 50998ec4e..08c8f776d 100644 --- a/doc/gallery-src/framework/run_grids3_rzt.py +++ b/doc/gallery-src/framework/run_grids3_rzt.py @@ -14,6 +14,7 @@ """ Make a Theta-R-Z grid. +====================== This builds a 3-D grid in Theta-R-Z geometry by specifying the theta, r, and z dimension bounds explicitly. diff --git a/doc/gallery-src/framework/run_isotxs.py b/doc/gallery-src/framework/run_isotxs.py index 1aa60da98..b0dbc50fa 100644 --- a/doc/gallery-src/framework/run_isotxs.py +++ b/doc/gallery-src/framework/run_isotxs.py @@ -13,6 +13,7 @@ # limitations under the License. """ Plotting Multi-group XS from ISOTXS. +==================================== In this example, several cross sections are plotted from an existing binary cross section library file in :py:mod:`ISOTXS ` format. diff --git a/doc/gallery-src/framework/run_isotxs2_matrix.py b/doc/gallery-src/framework/run_isotxs2_matrix.py index 6067c26e1..e197d9855 100644 --- a/doc/gallery-src/framework/run_isotxs2_matrix.py +++ b/doc/gallery-src/framework/run_isotxs2_matrix.py @@ -13,6 +13,7 @@ # limitations under the License. """ Plotting a multi-group scatter matrix. +====================================== Here we plot scatter matrices from an ISOTXS microscopic cross section library. We plot the inelastic scatter cross section of U235 as well as the (n,2n) source diff --git a/doc/gallery-src/framework/run_materials.py b/doc/gallery-src/framework/run_materials.py index 84fe2a6c9..bb78d4e8d 100644 --- a/doc/gallery-src/framework/run_materials.py +++ b/doc/gallery-src/framework/run_materials.py @@ -13,6 +13,7 @@ # limitations under the License. """ Listing of Material Library. +============================ This is a listing of all the elements in all the materials that are included in the ARMI material library. Many of the materials in this library are academic in quality and diff --git a/doc/gallery-src/framework/run_programmaticReactorDefinition.py b/doc/gallery-src/framework/run_programmaticReactorDefinition.py index 98d8c4dd6..e6fc92d28 100644 --- a/doc/gallery-src/framework/run_programmaticReactorDefinition.py +++ b/doc/gallery-src/framework/run_programmaticReactorDefinition.py @@ -13,6 +13,7 @@ # limitations under the License. """ Build Reactor Inputs Programmatically. +====================================== Sometimes it's desirable to build input definitions for ARMI using code rather than by writing the textual input files directly. diff --git a/doc/gallery-src/framework/run_reactorFacemap.py b/doc/gallery-src/framework/run_reactorFacemap.py index e4553c1d1..b00c22d27 100644 --- a/doc/gallery-src/framework/run_reactorFacemap.py +++ b/doc/gallery-src/framework/run_reactorFacemap.py @@ -13,6 +13,7 @@ # limitations under the License. """ Plot a reactor facemap. +======================= Load a test reactor from the test suite and plot a dummy power distribution from it. You can plot any block parameter. diff --git a/doc/gallery-src/framework/run_transmutationMatrix.py b/doc/gallery-src/framework/run_transmutationMatrix.py index 1f57eac8e..2546c65f1 100644 --- a/doc/gallery-src/framework/run_transmutationMatrix.py +++ b/doc/gallery-src/framework/run_transmutationMatrix.py @@ -13,6 +13,7 @@ # limitations under the License. """ Transmutation and decay reactions. +================================== This plots some of the transmutation and decay pathways for the actinides and some light nuclides using the burn chain definition that is included with ARMI. Note that many of diff --git a/doc/requirements-docs.txt b/doc/requirements-docs.txt index 09f83334c..fccddce61 100644 --- a/doc/requirements-docs.txt +++ b/doc/requirements-docs.txt @@ -1,41 +1,46 @@ # these are most specified that usual, because Sphinx docs seem to be quite fragile -Sphinx==4.4.0 +# limited <7 by sphinx-rtd-theme at the moment. +# sphinx-rtd-theme requires docutils <0.19 but sphinx dropped support for 0.18 in 6.0.0 +# so we're stuck at these versions +Sphinx==5.3.0 +docutils==0.18.1 + +# Read-The-Docs theme for Sphinx +sphinx-rtd-theme==1.2.2 # Parses Jupyter notebooks -nbsphinx==0.8.8 +nbsphinx==0.9.2 # Includes Jupyter notebooks in Sphinx source root nbsphinx-link==1.3.0 # builds a HTML version of a Python script and puts it into a gallery -sphinx-gallery==0.9.0 - -# Read-The-Docs theme for Sphinx -sphinx-rtd-theme==0.5.2 +sphinx-gallery==0.13.0 # allows us to more easily document our API sphinxcontrib-apidoc==0.3.0 -# generates OpenGraph metadata? -sphinxext-opengraph==0.5.1 +# generates OpenGraph metadata to make good-looking cards on social media +sphinxext-opengraph==0.8.2 -# supporting requirement documentation -sphinxcontrib-needs==0.7.6 -sphinxcontrib-plantuml==0.23 +# supporting requirements traceability matrices for QA +sphinx-needs==1.2.2 + +# uml support in sphinx-needs +sphinxcontrib-plantuml==0.25 # This is not strictly necessary via PIP, but MUST be in the path. # Used to convert between file formats. pandoc # iPython kernel to run Jupyter notebooks -ipykernel==6.7.0 - -# docutils 0.17 introduced a bug that prevents bullets from rendering -# see https://github.com/terrapower/armi/issues/274 -docutils <0.17 +ipykernel==6.25.1 # used to generate UML diagrams -pylint==2.7.4 +pylint==2.17.5 + +# used in numpydoc and nbconvert +Jinja2==3.0.3 -# needed to side step bug: https://github.com/numpy/numpydoc/issues/380 -Jinja2==3.0.1 +# deal with missing jquery errors +sphinxcontrib-jquery==4.1 diff --git a/doc/requirements/srsd.rst b/doc/requirements/srsd.rst index 8ac12e6e6..26039c939 100644 --- a/doc/requirements/srsd.rst +++ b/doc/requirements/srsd.rst @@ -510,7 +510,7 @@ TODO: This is completed by the :doc:`Settings Report Date: Mon, 14 Aug 2023 18:56:12 -0700 Subject: [PATCH 08/17] Removing unnecessary f-strings (#1387) --- armi/cases/case.py | 6 +++--- armi/cli/tests/test_runEntryPoint.py | 4 ++-- armi/interfaces.py | 2 +- armi/nucDirectory/nuclideBases.py | 2 +- armi/nucDirectory/transmutations.py | 2 +- armi/nuclearDataIO/xsLibraries.py | 4 ++-- armi/physics/fuelPerformance/parameters.py | 4 ++-- .../physics/neutronics/crossSectionGroupManager.py | 2 +- .../fissionProductModel/lumpedFissionProduct.py | 4 ++-- .../latticePhysics/latticePhysicsInterface.py | 2 +- armi/physics/thermalHydraulics/settings.py | 2 +- armi/reactor/blocks.py | 2 +- armi/reactor/blueprints/gridBlueprint.py | 2 +- armi/reactor/blueprints/isotopicOptions.py | 6 +++--- armi/reactor/components/__init__.py | 4 ++-- armi/reactor/converters/axialExpansionChanger.py | 4 ++-- armi/reactor/converters/uniformMesh.py | 14 +++++++------- armi/settings/fwSettings/globalSettings.py | 8 ++++---- 18 files changed, 37 insertions(+), 37 deletions(-) diff --git a/armi/cases/case.py b/armi/cases/case.py index 19658ed18..7ad725320 100644 --- a/armi/cases/case.py +++ b/armi/cases/case.py @@ -545,8 +545,8 @@ def _initBurnChain(self): if not os.path.exists(self.cs["burnChainFileName"]): raise ValueError( f"The burn-chain file {self.cs['burnChainFileName']} does not exist. The " - f"data cannot be loaded. Fix this path or disable burn-chain initialization using " - f"the `initializeBurnChain` setting." + "data cannot be loaded. Fix this path or disable burn-chain initialization using " + "the `initializeBurnChain` setting." ) with open(self.cs["burnChainFileName"]) as burnChainStream: @@ -939,7 +939,7 @@ def copyInterfaceInputs( except NonexistentSetting(key): raise ValueError( f"{key} is not a valid setting. Ensure the relevant specifyInputs " - f"method uses a correct setting name." + "method uses a correct setting name." ) label = key.name diff --git a/armi/cli/tests/test_runEntryPoint.py b/armi/cli/tests/test_runEntryPoint.py index 0838bde66..069ad20e3 100644 --- a/armi/cli/tests/test_runEntryPoint.py +++ b/armi/cli/tests/test_runEntryPoint.py @@ -56,8 +56,8 @@ def test_entryPointInitialization(self): settingsArg, msg=( f"A settings file argument was expected for {entryPoint}, " - f"but does not exist. This is a error in the EntryPoint " - f"implementation." + "but does not exist. This is a error in the EntryPoint " + "implementation." ), ) diff --git a/armi/interfaces.py b/armi/interfaces.py index f9f6b4c03..5d48075e1 100644 --- a/armi/interfaces.py +++ b/armi/interfaces.py @@ -169,7 +169,7 @@ def isConverged(self, val: _SUPPORTED_TYPES) -> bool: if self._previousIterationValue is None: raise ValueError( f"Cannot check convergence of {self} with no previous iteration value set. " - f"Set using `storePreviousIterationValue` first." + "Set using `storePreviousIterationValue` first." ) previous = self._previousIterationValue diff --git a/armi/nucDirectory/nuclideBases.py b/armi/nucDirectory/nuclideBases.py index aeafee524..3cc9427a3 100644 --- a/armi/nucDirectory/nuclideBases.py +++ b/armi/nucDirectory/nuclideBases.py @@ -407,7 +407,7 @@ def _processBurnData(self, burnInfo): runLog.info( f"nuSF provided for {self} will be updated from " f"{self.nuSF:<8.6e} to {userSpontaneousFissionYield:<8.6e} based on " - f"user provided burn-chain data." + "user provided burn-chain data." ) self.nuSF = userSpontaneousFissionYield else: diff --git a/armi/nucDirectory/transmutations.py b/armi/nucDirectory/transmutations.py index b1e94ff38..fe2c103f2 100644 --- a/armi/nucDirectory/transmutations.py +++ b/armi/nucDirectory/transmutations.py @@ -229,7 +229,7 @@ def __init__(self, parent, dataDict): runLog.info( f"Half-life provided for {self} will be updated from " f"{parent.halflife:<15.11e} to {userHalfLife:<15.11e} seconds based on " - f"user provided burn-chain data." + "user provided burn-chain data." ) self.halfLifeInSeconds = userHalfLife diff --git a/armi/nuclearDataIO/xsLibraries.py b/armi/nuclearDataIO/xsLibraries.py index 570296e9e..126437761 100644 --- a/armi/nuclearDataIO/xsLibraries.py +++ b/armi/nuclearDataIO/xsLibraries.py @@ -264,9 +264,9 @@ def mergeXSLibrariesInWorkingDirectory( os.path.exists(gamisoLibraryPath) and os.path.exists(pmatrxLibraryPath) ): runLog.warning( - f"One of GAMISO or PMATRX data exist for " + "One of GAMISO or PMATRX data exist for " f"XS ID {xsID} with suffix {xsLibrarySuffix}. " - f"Attempting to find GAMISO/PMATRX data with " + "Attempting to find GAMISO/PMATRX data with " f"only XS ID {xsID} instead." ) gamisoLibraryPath = os.path.join( diff --git a/armi/physics/fuelPerformance/parameters.py b/armi/physics/fuelPerformance/parameters.py index 7622c2daa..604a37015 100644 --- a/armi/physics/fuelPerformance/parameters.py +++ b/armi/physics/fuelPerformance/parameters.py @@ -39,7 +39,7 @@ def _getFuelPerformanceBlockParams(): def gasReleaseFraction(self, value): if value < 0.0 or value > 1.0: raise ValueError( - f"Cannot set a gas release fraction " + "Cannot set a gas release fraction " f"of {value} outside of the bounds of [0.0, 1.0]" ) self._p_gasReleaseFraction = value @@ -55,7 +55,7 @@ def gasReleaseFraction(self, value): def bondRemoved(self, value): if value < 0.0 or value > 1.0: raise ValueError( - f"Cannot set a bond removed " + "Cannot set a bond removed " f"of {value} outside of the bounds of [0.0, 1.0]" ) self._p_bondRemoved = value diff --git a/armi/physics/neutronics/crossSectionGroupManager.py b/armi/physics/neutronics/crossSectionGroupManager.py index 5ba6e91be..3c8162177 100644 --- a/armi/physics/neutronics/crossSectionGroupManager.py +++ b/armi/physics/neutronics/crossSectionGroupManager.py @@ -466,7 +466,7 @@ def _checkComponentConsistency(b, repBlock): if len(b) != len(repBlock): raise ValueError( f"Blocks {b} and {repBlock} have differing number " - f"of components and cannot be homogenized" + "of components and cannot be homogenized" ) # Using Fe-56 as a proxy for structure and Na-23 as proxy for coolant is undesirably SFR-centric # This should be generalized in the future, if possible diff --git a/armi/physics/neutronics/fissionProductModel/lumpedFissionProduct.py b/armi/physics/neutronics/fissionProductModel/lumpedFissionProduct.py index 9cf1127ab..3610b2ca7 100644 --- a/armi/physics/neutronics/fissionProductModel/lumpedFissionProduct.py +++ b/armi/physics/neutronics/fissionProductModel/lumpedFissionProduct.py @@ -92,7 +92,7 @@ def __setitem__(self, key, val): if val < 0.0: raise ValueError( f"Cannot set the yield of {key} in {self} to be " - f"less than zero as this is non-physical." + "less than zero as this is non-physical." ) if val > NUM_FISSION_PRODUCTS_PER_LFP: raise ValueError( @@ -387,7 +387,7 @@ def lumpedFissionProductFactory(cs): lfpPath = cs[CONF_LFP_COMPOSITION_FILE_PATH] if not lfpPath or not os.path.exists(lfpPath): raise ValueError( - f"The fission product reference file does " + "The fission product reference file does " f"not exist or is not a valid path. Path provided: {lfpPath}" ) diff --git a/armi/physics/neutronics/latticePhysics/latticePhysicsInterface.py b/armi/physics/neutronics/latticePhysics/latticePhysicsInterface.py index 67de67e52..f03c21af5 100644 --- a/armi/physics/neutronics/latticePhysics/latticePhysicsInterface.py +++ b/armi/physics/neutronics/latticePhysics/latticePhysicsInterface.py @@ -425,7 +425,7 @@ def _newLibraryShouldBeCreated(self, cycle, representativeBlockList, xsIDs): runLog.info( f"Although a XS library {self.r.core._lib} exists on {self.r.core}, " f"there are missing XS IDs {missing} required. The XS generation on cycle {cycle} " - f"is not enabled, but will be run to generate these missing cross sections." + "is not enabled, but will be run to generate these missing cross sections." ) executeXSGen = True elif missing: diff --git a/armi/physics/thermalHydraulics/settings.py b/armi/physics/thermalHydraulics/settings.py index 5dd76b20e..a8df91ed2 100644 --- a/armi/physics/thermalHydraulics/settings.py +++ b/armi/physics/thermalHydraulics/settings.py @@ -28,7 +28,7 @@ def defineSettings(): default=False, label="Run Thermal Hydraulics", description=( - f"Activate thermal hydraulics calculations using the physics module defined in " + "Activate thermal hydraulics calculations using the physics module defined in " f"`{CONF_TH_KERNEL}`" ), ), diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py index 326b03f0c..f068cc000 100644 --- a/armi/reactor/blocks.py +++ b/armi/reactor/blocks.py @@ -847,7 +847,7 @@ def setB10VolParam(self, heightHot): runLog.warning( f"More than one boron10-containing component found in {self.name}. " f"Only {b10Comp} will be considered for calculation of initialB10ComponentVol " - f"Since adding multiple volumes is not conservative for captures/cc." + "Since adding multiple volumes is not conservative for captures/cc." f"All compos found {b10Comps}", single=True, ) diff --git a/armi/reactor/blueprints/gridBlueprint.py b/armi/reactor/blueprints/gridBlueprint.py index d6eb86bac..1cc6eb1d7 100644 --- a/armi/reactor/blueprints/gridBlueprint.py +++ b/armi/reactor/blueprints/gridBlueprint.py @@ -574,7 +574,7 @@ def saveToStream(stream, bluep, full=False, tryMap=False): aMap.gridContentsToAscii() except Exception as e: runLog.warning( - f"The `lattice map` for the current assembly arrangement cannot be written. " + "The `lattice map` for the current assembly arrangement cannot be written. " f"Defaulting to using the `grid contents` dictionary instead. Exception: {e}" ) aMap = None diff --git a/armi/reactor/blueprints/isotopicOptions.py b/armi/reactor/blueprints/isotopicOptions.py index 993851589..0e37b4cfb 100644 --- a/armi/reactor/blueprints/isotopicOptions.py +++ b/armi/reactor/blueprints/isotopicOptions.py @@ -580,7 +580,7 @@ def autoUpdateNuclideFlags(cs, nuclideFlags, inerts): nbs = getAllNuclideBasesByLibrary(cs) if nbs: runLog.info( - f"Adding explicit fission products to the nuclide flags based on the " + "Adding explicit fission products to the nuclide flags based on the " f"fission product model set to `{cs[CONF_FP_MODEL]}`." ) for nb in nbs: @@ -605,8 +605,8 @@ def getAllNuclideBasesByLibrary(cs): nbs = nuclideBases.byMcc3Id.values() else: raise ValueError( - f"An option to handle the `CONF_FISSION_PRODUCT_LIBRARY_NAME` " + "An option to handle the `CONF_FISSION_PRODUCT_LIBRARY_NAME` " f"set to `{cs[CONF_FISSION_PRODUCT_LIBRARY_NAME]}` has not been " - f"implemented." + "implemented." ) return nbs diff --git a/armi/reactor/components/__init__.py b/armi/reactor/components/__init__.py index 26c4c06f5..9d3e940d2 100644 --- a/armi/reactor/components/__init__.py +++ b/armi/reactor/components/__init__.py @@ -376,8 +376,8 @@ def _deriveVolumeAndArea(self): if remainingVolume < 0: msg = ( f"The component areas in {self.parent} exceed the maximum " - f"allowable volume based on the geometry. Check that the " - f"geometry is defined correctly.\n" + "allowable volume based on the geometry. Check that the " + "geometry is defined correctly.\n" f"Maximum allowable volume: {self.parent.getMaxVolume()} cm^3\n" f"Volume of all non-derived shape components: {siblingVolume} cm^3\n" ) diff --git a/armi/reactor/converters/axialExpansionChanger.py b/armi/reactor/converters/axialExpansionChanger.py index 8e7f365d2..b3dd509a7 100644 --- a/armi/reactor/converters/axialExpansionChanger.py +++ b/armi/reactor/converters/axialExpansionChanger.py @@ -622,7 +622,7 @@ def setExpansionFactors(self, componentLst: List, expFrac: List): """ if len(componentLst) != len(expFrac): runLog.error( - f"Number of components and expansion fractions must be the same!\n" + "Number of components and expansion fractions must be the same!\n" f" len(componentLst) = {len(componentLst)}\n" f" len(expFrac) = {len(expFrac)}" ) @@ -801,7 +801,7 @@ def determineTargetComponent(self, b, flagOfInterest=None): raise RuntimeError(f"No target component found!\n Block {b}") if len(componentWFlag) > 1: raise RuntimeError( - f"Cannot have more than one component within a block that has the target flag!" + "Cannot have more than one component within a block that has the target flag!" f"Block {b}\nflagOfInterest {flagOfInterest}\nComponents {componentWFlag}" ) self._componentDeterminesBlockHeight[componentWFlag[0]] = True diff --git a/armi/reactor/converters/uniformMesh.py b/armi/reactor/converters/uniformMesh.py index 488696db2..64569046a 100644 --- a/armi/reactor/converters/uniformMesh.py +++ b/armi/reactor/converters/uniformMesh.py @@ -200,7 +200,7 @@ def _decuspAxialMesh(self): ) runLog.extra( - f"Attempting to honor control and fuel material boundaries in uniform mesh " + "Attempting to honor control and fuel material boundaries in uniform mesh " f"for {self} while also keeping minimum mesh size of {self.minimumMeshSize}. " f"Material boundaries are: {allMatBounds}" ) @@ -420,7 +420,7 @@ def convert(self, r=None): if self._hasNonUniformAssems: runLog.extra( f"Replacing non-uniform assemblies in reactor {r}, " - f"with assemblies whose axial mesh is uniform with " + "with assemblies whose axial mesh is uniform with " f"the core's reference assembly mesh: {r.core.refAssem.getAxialMesh()}" ) self.convReactor = self._sourceReactor @@ -560,8 +560,8 @@ def applyStateToOriginal(self): runLog.error( f"No assembly matching name {assem.getName()} " f"was found in the temporary storage list. {assem} " - f"will persist as an axially unified assembly. " - f"This is likely not intended." + "will persist as an axially unified assembly. " + "This is likely not intended." ) self._sourceReactor.core.updateAxialMesh() @@ -775,12 +775,12 @@ def setAssemblyStateFromOverlaps( continue elif not sourceBlocksInfo: raise ValueError( - f"An error occurred when attempting to map to the " + "An error occurred when attempting to map to the " f"results from {sourceAssembly} to {destinationAssembly}. " f"No blocks in {sourceAssembly} exist between the axial " f"elevations of {zLower:<12.5f} cm and {zUpper:<12.5f} cm. " - f"This a major bug in the uniform mesh converter that should " - f"be reported to the developers." + "This a major bug in the uniform mesh converter that should " + "be reported to the developers." ) if mapNumberDensities: diff --git a/armi/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py index 18ff27432..16a126ffb 100644 --- a/armi/settings/fwSettings/globalSettings.py +++ b/armi/settings/fwSettings/globalSettings.py @@ -142,10 +142,10 @@ def defineSettings() -> List[setting.Setting]: label="Initialize Burn Chain", description=( f"This setting is paired with the `{CONF_BURN_CHAIN_FILE_NAME}` setting. " - f"When enabled, this will initialize the burn-chain on initializing the case and " - f"is required for running depletion calculations where the transmutations and decays " - f"are controlled by the framework. If an external software, such as ORIGEN, contains " - f"data for the burn-chain already embedded then this may be disabled." + "When enabled, this will initialize the burn-chain on initializing the case and " + "is required for running depletion calculations where the transmutations and decays " + "are controlled by the framework. If an external software, such as ORIGEN, contains " + "data for the burn-chain already embedded then this may be disabled." ), ), setting.Setting( From 37263cd2e8158fdba11843c271d47d97f8e98dcf Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Wed, 16 Aug 2023 18:36:24 -0700 Subject: [PATCH 09/17] Removing unreachable code block (#1391) --- armi/reactor/blocks.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py index f068cc000..a7a115501 100644 --- a/armi/reactor/blocks.py +++ b/armi/reactor/blocks.py @@ -1965,13 +1965,6 @@ def verifyBlockDims(self): pinToDuctGap, self ) ) - wire = self.getComponent(Flags.WIRE) - wireThicknesses = wire.getDimension("od", cold=False) - if pinToDuctGap < wireThicknesses: - raise ValueError( - "Gap between pins and duct is {0:.4f} cm in {1} which does not allow room for the wire " - "with diameter {2}".format(pinToDuctGap, self, wireThicknesses) - ) elif pinToDuctGap is None: # only produce a warning if pin or clad are found, but not all of pin, clad and duct. We # may need to tune this logic a bit From 57a2f09bcd33cf15f27a6b3b86b4f9089b2586b8 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 08:40:08 -0700 Subject: [PATCH 10/17] Fixing broken link (#1390) --- armi/reactor/parameters/parameterDefinitions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/armi/reactor/parameters/parameterDefinitions.py b/armi/reactor/parameters/parameterDefinitions.py index e1163ebde..5d43b90b1 100644 --- a/armi/reactor/parameters/parameterDefinitions.py +++ b/armi/reactor/parameters/parameterDefinitions.py @@ -38,7 +38,7 @@ from armi.reactor.parameters.exceptions import ParameterError, ParameterDefinitionError # bitwise masks for high-speed operations on the `assigned` attribute -# (see http://www.vipan.com/htdocs/bitwisehelp.html) +# see: https://web.archive.org/web/20120225043338/http://www.vipan.com/htdocs/bitwisehelp.html # Note that the various operations are responsible for clearing the flags on the events. # These should be interpreted as: # The Parameter or ParameterCollection has been modified SINCE_ From df6d76414952d263a65b455d82c872251911b526 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 09:09:06 -0700 Subject: [PATCH 11/17] Fixing ruff formatting of doc gallery examples (#1389) --- ruff.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.toml b/ruff.toml index dc9e32237..402d61c61 100644 --- a/ruff.toml +++ b/ruff.toml @@ -76,6 +76,7 @@ line-length = 88 # N - We have our own naming conventions for unit tests. # SLF001 - private member access "*/tests/*" = ["D1", "E741", "N", "SLF001"] +"doc/gallery-src/*" = ["D400"] [pydocstyle] # Use numpy-style docstrings. From 21db2d5aba34b2f6a6e6158b8e1825049ef9389d Mon Sep 17 00:00:00 2001 From: Michael Jarrett Date: Thu, 17 Aug 2023 09:59:52 -0700 Subject: [PATCH 12/17] Update reactivity coefficient parameters (#1355) * Update rxcoeff parameter attributes. * Add thermal hydraulics parameter category. * Add rxFuelAxialExpansionPerPercent parameter. --- armi/physics/thermalHydraulics/parameters.py | 5 ++++- armi/reactor/blockParameters.py | 2 +- .../converters/tests/test_uniformMesh.py | 8 +++---- armi/reactor/converters/uniformMesh.py | 21 +++++-------------- .../parameters/parameterDefinitions.py | 4 ++++ armi/reactor/reactorParameters.py | 6 ++++++ doc/release/0.2.rst | 1 + 7 files changed, 25 insertions(+), 22 deletions(-) diff --git a/armi/physics/thermalHydraulics/parameters.py b/armi/physics/thermalHydraulics/parameters.py index 16fdc2727..88e6384ec 100644 --- a/armi/physics/thermalHydraulics/parameters.py +++ b/armi/physics/thermalHydraulics/parameters.py @@ -268,7 +268,7 @@ def _getBlockParams(): ) with pDefs.createBuilder( - default=0.0, categories=["thermal hydraulics", "mongoose"], saveToDB=True + default=None, categories=["thermal hydraulics", "mongoose"], saveToDB=True ) as pb: pb.defParam( @@ -285,6 +285,9 @@ def _getBlockParams(): location=ParamLocation.TOP | ParamLocation.EDGES, ) + with pDefs.createBuilder( + default=0, categories=["thermal hydraulics", "mongoose"], saveToDB=True + ) as pb: pb.defParam( "THhotChannel", units=units.UNITLESS, diff --git a/armi/reactor/blockParameters.py b/armi/reactor/blockParameters.py index 06ac8d020..478c2cda8 100644 --- a/armi/reactor/blockParameters.py +++ b/armi/reactor/blockParameters.py @@ -704,7 +704,7 @@ def xsTypeNum(self, value): with pDefs.createBuilder( default=0.0, - location=ParamLocation.AVERAGE, + location=ParamLocation.VOLUME_INTEGRATED, categories=[ "reactivity coefficients", "spatially dependent", diff --git a/armi/reactor/converters/tests/test_uniformMesh.py b/armi/reactor/converters/tests/test_uniformMesh.py index 35fc66a3b..fb1ad27a6 100644 --- a/armi/reactor/converters/tests/test_uniformMesh.py +++ b/armi/reactor/converters/tests/test_uniformMesh.py @@ -415,8 +415,8 @@ def test_applyStateToOriginal(self): self.converter.convert(self.r) for ib, b in enumerate(self.converter.convReactor.core.getBlocks()): - b.p.mgFlux = range(33) - b.p.adjMgFlux = range(33) + b.p.mgFlux = list(range(33)) + b.p.adjMgFlux = list(range(33)) b.p.fastFlux = 2.0 b.p.flux = 5.0 b.p.power = 5.0 @@ -507,8 +507,8 @@ def test_applyStateToOriginal(self): # set original parameters on pre-mapped core with non-uniform assemblies for b in self.r.core.getBlocks(): - b.p.mgFlux = range(33) - b.p.adjMgFlux = range(33) + b.p.mgFlux = list(range(33)) + b.p.adjMgFlux = list(range(33)) b.p.fastFlux = 2.0 b.p.flux = 5.0 b.p.power = 5.0 diff --git a/armi/reactor/converters/uniformMesh.py b/armi/reactor/converters/uniformMesh.py index 64569046a..a89349ace 100644 --- a/armi/reactor/converters/uniformMesh.py +++ b/armi/reactor/converters/uniformMesh.py @@ -1317,7 +1317,7 @@ def paramSetter(block, vals, paramNames): if val is None: continue - if isinstance(val, (list, numpy.ndarray)): + if isinstance(val, (tuple, list, numpy.ndarray)): ParamMapper._arrayParamSetter(block, [val], [paramName]) else: ParamMapper._scalarParamSetter(block, [val], [paramName]) @@ -1327,22 +1327,11 @@ def paramGetter(self, block, paramNames): paramVals = [] for paramName in paramNames: val = block.p[paramName] - defaultValue = self.paramDefaults[paramName] - valType = type(defaultValue) - # Array / list parameters can be have values that are `None`, lists, or numpy arrays. This first - # checks if the value type is any of these and if so, the block-level parameter is treated as an - # array. - if isinstance(valType, (list, numpy.ndarray)) or isinstance(None, valType): - if val is None or len(val) == 0: - paramVals.append(None) - else: - paramVals.append(numpy.array(val)) - # Otherwise, the parameter is treated as a scalar, like a float/string/integer. + # list-like should be treated as a numpy array + if isinstance(val, (tuple, list, numpy.ndarray)): + paramVals.append(numpy.array(val) if len(val) > 0 else None) else: - if val == defaultValue: - paramVals.append(defaultValue) - else: - paramVals.append(val) + paramVals.append(val) return numpy.array(paramVals, dtype=object) diff --git a/armi/reactor/parameters/parameterDefinitions.py b/armi/reactor/parameters/parameterDefinitions.py index 5d43b90b1..458a849fc 100644 --- a/armi/reactor/parameters/parameterDefinitions.py +++ b/armi/reactor/parameters/parameterDefinitions.py @@ -70,6 +70,8 @@ class Category: * `neutronics` parameters are calculated in a neutronics global flux solve * `gamma` parameters are calculated in a fixed-source gamma solve * `detailedAxialExpansion` parameters are marked as such so that they are mapped from the uniform mesh back to the non-uniform mesh + * `reactivity coefficients` parameters are related to reactivity coefficient or kinetics parameters for kinetics solutions + * `thermal hydraulics` parameters come from a thermal hydraulics physics plugin (e.g., flow rates, temperatures, etc.) """ depletion = "depletion" @@ -83,6 +85,8 @@ class Category: neutronics = "neutronics" gamma = "gamma" detailedAxialExpansion = "detailedAxialExpansion" + reactivityCoefficients = "reactivity coefficients" + thermalHydraulics = "thermal hydraulics" class ParamLocation(enum.Flag): diff --git a/armi/reactor/reactorParameters.py b/armi/reactor/reactorParameters.py index 570d54297..2fe1bc5bc 100644 --- a/armi/reactor/reactorParameters.py +++ b/armi/reactor/reactorParameters.py @@ -629,6 +629,12 @@ def defineCoreParameters(): description="Fuel Axial Expansion Coefficient", ) + pb.defParam( + "rxFuelAxialExpansionCoeffPerPercent", + units="dk/kk'-%", + description="Fuel Axial Expansion Coefficient", + ) + pb.defParam( "rxGridPlateRadialExpansionCoeffPerTemp", units=f"{units.REACTIVITY}/{units.DEGK}", diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst index 545ad762c..44065b86c 100644 --- a/doc/release/0.2.rst +++ b/doc/release/0.2.rst @@ -15,6 +15,7 @@ What's new in ARMI #. Add python 3.11 to ARMI's CI testing GH actions! (`PR#1341 `_) #. Put back ``avgFuelTemp`` block parameter. (`PR#1362 `_) #. Make cylindrical component block collection less strict about pre-homogenization checks. (`PR#1347 `_) +#. Updated some parameter definitions and defaults. (`PR#1355 `_) #. TBD Bug fixes From 0f32729b7504da8e8e676093eed2bedfa012ace1 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 10:31:20 -0700 Subject: [PATCH 13/17] Removing global plugin flags (#1388) --- armi/__init__.py | 2 +- armi/apps.py | 19 +++++++++++++++++++ armi/reactor/flags.py | 23 ----------------------- armi/tests/test_apps.py | 15 +++++++++++++++ 4 files changed, 35 insertions(+), 24 deletions(-) diff --git a/armi/__init__.py b/armi/__init__.py index a3471516b..dff87ddd3 100644 --- a/armi/__init__.py +++ b/armi/__init__.py @@ -307,7 +307,7 @@ def configure(app: Optional[apps.App] = None, permissive=False): pm = app.pluginManager parameters.collectPluginParameters(pm) parameters.applyAllParameters() - flags.registerPluginFlags(pm) + _app.registerPluginFlags() def applyAsyncioWindowsWorkaround() -> None: diff --git a/armi/apps.py b/armi/apps.py index 1e748312f..bce6cd63f 100644 --- a/armi/apps.py +++ b/armi/apps.py @@ -75,6 +75,7 @@ def __init__(self): For a description of the things that an ARMI plugin can do, see the :py:mod:`armi.plugins` module. """ + self._pluginFlagsRegistered: bool = False self._pm: Optional[pluginManager.ArmiPluginManager] = None self._paramRenames: Optional[Tuple[Dict[str, str], int]] = None self.__initNewPlugins() @@ -228,6 +229,24 @@ def getParamRenames(self) -> Dict[str, str]: self._paramRenames = renames, self._pm.counter return renames + def registerPluginFlags(self): + """ + Apply flags specified in the passed ``PluginManager`` to the ``Flags`` class. + + See Also + -------- + armi.plugins.ArmiPlugin.defineFlags + """ + if self._pluginFlagsRegistered: + raise RuntimeError( + "Plugin flags have already been registered. Cannot do it twice!" + ) + + for pluginFlags in self._pm.hook.defineFlags(): + Flags.extend(pluginFlags) + + self._pluginFlagsRegistered = True + def registerUserPlugins(self, pluginPaths): r""" Register additional plugins passed in by importable paths. diff --git a/armi/reactor/flags.py b/armi/reactor/flags.py index d1e2c8e34..c59775a80 100644 --- a/armi/reactor/flags.py +++ b/armi/reactor/flags.py @@ -151,7 +151,6 @@ def _fromStringIgnoreErrors(cls, typeSpec): a. multiple-word flags are used such as *grid plate* or *inlet nozzle* so we use lookups. b. Some flags have digits in them. We just strip those off. - """ def updateMethodIgnoreErrors(typeSpec): @@ -298,28 +297,6 @@ class InvalidFlagsError(KeyError): pass -_PLUGIN_FLAGS_REGISTERED = False - - -def registerPluginFlags(pm): - """ - Apply flags specified in the passed ``PluginManager`` to the ``Flags`` class. - - See Also - -------- - armi.plugins.ArmiPlugin.defineFlags - """ - global _PLUGIN_FLAGS_REGISTERED - if _PLUGIN_FLAGS_REGISTERED: - raise RuntimeError( - "Plugin flags have already been registered. Cannot do it twice!" - ) - - for pluginFlags in pm.hook.defineFlags(): - Flags.extend(pluginFlags) - _PLUGIN_FLAGS_REGISTERED = True - - # string conversions for multiple-word flags # Beware of how these may interact with the standard flag names! E.g., make sure NOZZLE # doesn't eat the NOZZLE in INLET_NOZZLE. Make sure that words that would otherwise be a diff --git a/armi/tests/test_apps.py b/armi/tests/test_apps.py index 779b1eff6..a353b1ab3 100644 --- a/armi/tests/test_apps.py +++ b/armi/tests/test_apps.py @@ -24,6 +24,8 @@ from armi import isStableReleaseVersion from armi import meta from armi import plugins +from armi import utils +from armi.reactor.flags import Flags from armi.__main__ import main @@ -119,6 +121,19 @@ def test_getParamRenames(self): ): app.getParamRenames() + def test_registerPluginFlags(self): + # set up the app, pm, and register some plugins + app = getApp() + + # validate our flags have been registered + self.assertEqual(Flags.fromString("FUEL"), Flags.FUEL) + self.assertEqual(Flags.fromString("PRIMARY"), Flags.PRIMARY) + + # validate we can only register the flags once + for _ in range(3): + with self.assertRaises(RuntimeError): + app.registerPluginFlags() + def test_getParamRenamesInvalids(self): # a basic test of the method app = getApp() From 5403ec2d854af27d28477d33bef46535cc7a5f89 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:19:12 -0700 Subject: [PATCH 14/17] Fixing some miscellaneous TODOs (#1392) --- armi/bookkeeping/db/databaseInterface.py | 1 - armi/bookkeeping/db/layout.py | 11 ++--------- armi/bookkeeping/memoryProfiler.py | 6 +----- armi/reactor/blocks.py | 6 +++++- armi/reactor/blueprints/assemblyBlueprint.py | 1 - armi/reactor/composites.py | 3 +-- armi/reactor/grids.py | 7 ++----- armi/reactor/tests/test_grids.py | 8 ++++++++ armi/settings/fwSettings/globalSettings.py | 3 --- 9 files changed, 19 insertions(+), 27 deletions(-) diff --git a/armi/bookkeeping/db/databaseInterface.py b/armi/bookkeeping/db/databaseInterface.py index 8fb4e2881..d49f04aec 100644 --- a/armi/bookkeeping/db/databaseInterface.py +++ b/armi/bookkeeping/db/databaseInterface.py @@ -281,7 +281,6 @@ def _checkThatCyclesHistoriesAreEquivalentUpToRestartTime( "The cycle history up to the restart cycle/node must be equivalent." ) - # TODO: The use of "yield" here is suspect. def _getLoadDB(self, fileName): """ Return the database to load from in order of preference. diff --git a/armi/bookkeeping/db/layout.py b/armi/bookkeeping/db/layout.py index 199c47250..c5bb92498 100644 --- a/armi/bookkeeping/db/layout.py +++ b/armi/bookkeeping/db/layout.py @@ -356,7 +356,7 @@ def _initComps(self, caseTitle, bp): elif issubclass(Klass, Core): comp = Klass(name) elif issubclass(Klass, Component): - # TODO: init all dimensions to 0, they will be loaded and assigned after load + # init all dimensions to 0, they will be loaded and assigned after load kwargs = dict.fromkeys(Klass.DIMENSION_NAMES, 0) kwargs["material"] = material kwargs["name"] = name @@ -851,18 +851,11 @@ def replaceNonsenseWithNones(data: numpy.ndarray, paramName: str) -> numpy.ndarr if isNone[i].all(): result[i] = None elif isNone[i].any(): - # TODO: This is not symmetric with the replaceNonesWithNonsense impl. - # That one assumes that Nones apply only at the highest dimension, and - # that the lower dimensions will be filled with the magic None value. - # Non-none entries below the top level fail to coerce to a serializable - # numpy array and would raise an exception when trying to write. TL;DR: - # this is a dead branch until the replaceNonesWithNonsense impl is more - # sophisticated. + # This is the meat of the logic to replace "nonsense" with None. result[i] = numpy.array(data[i], dtype=numpy.dtype("O")) result[i][isNone[i]] = None else: result[i] = data[i] - else: result = numpy.ndarray(data.shape, dtype=numpy.dtype("O")) result[:] = data diff --git a/armi/bookkeeping/memoryProfiler.py b/armi/bookkeeping/memoryProfiler.py index 5997249f2..6a1053fe2 100644 --- a/armi/bookkeeping/memoryProfiler.py +++ b/armi/bookkeeping/memoryProfiler.py @@ -307,11 +307,7 @@ def add(self, item): self.ids.add(itemId) if self.reportSize: - try: - self.memSize += sys.getsizeof(item) - except: # noqa: bare-except - # TODO: Does this happen? - self.memSize = float("nan") + self.memSize += sys.getsizeof(item) self.count += 1 return True diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py index a7a115501..1b2322766 100644 --- a/armi/reactor/blocks.py +++ b/armi/reactor/blocks.py @@ -562,6 +562,8 @@ def adjustUEnrich(self, newEnrich): newEnrich : float New U-235 enrichment in mass fraction + Notes + ----- completeInitialLoading must be run because adjusting the enrichment actually changes the mass slightly and you can get negative burnups, which you do not want. """ @@ -800,7 +802,6 @@ def completeInitialLoading(self, bolBlock=None): hmDens = bolBlock.getHMDens() # total homogenized heavy metal number density self.p.nHMAtBOL = hmDens - self.p.molesHmBOL = self.getHMMoles() self.p.puFrac = ( self.getPuMoles() / self.p.molesHmBOL if self.p.molesHmBOL > 0.0 else 0.0 @@ -811,6 +812,7 @@ def completeInitialLoading(self, bolBlock=None): self.p.smearDensity = self.getSmearDensity() except ValueError: pass + self.p.enrichmentBOL = self.getFissileMassEnrich() massHmBOL = 0.0 sf = self.getSymmetryFactor() @@ -820,7 +822,9 @@ def completeInitialLoading(self, bolBlock=None): # Components have a massHmBOL parameter but not every composite will if isinstance(child, components.Component): child.p.massHmBOL = hmMass + self.p.massHmBOL = massHmBOL + return hmDens def setB10VolParam(self, heightHot): diff --git a/armi/reactor/blueprints/assemblyBlueprint.py b/armi/reactor/blueprints/assemblyBlueprint.py index b0d80b930..727437cea 100644 --- a/armi/reactor/blueprints/assemblyBlueprint.py +++ b/armi/reactor/blueprints/assemblyBlueprint.py @@ -214,7 +214,6 @@ def _createBlock(self, cs, blueprint, bDesign, axialIndex): cs, blueprint, axialIndex, meshPoints, height, xsType, materialInput ) - # TODO: remove when the plugin system is fully set up? b.completeInitialLoading() # set b10 volume cc since its a cold dim param diff --git a/armi/reactor/composites.py b/armi/reactor/composites.py index 1c3c1abd9..92b5ad297 100644 --- a/armi/reactor/composites.py +++ b/armi/reactor/composites.py @@ -392,8 +392,7 @@ def __getstate__(self): state["parent"] = None if "r" in state: - # TODO: This should never happen, it might make sense to raise an exception. - del state["r"] + raise RuntimeError("An ArmiObject should never contain the entire Reactor.") return state diff --git a/armi/reactor/grids.py b/armi/reactor/grids.py index 9ce755450..6c9d7c1ce 100644 --- a/armi/reactor/grids.py +++ b/armi/reactor/grids.py @@ -1054,8 +1054,7 @@ def getRingPos(self, indices) -> Tuple[int, int]: A tuple is returned so that it is easy to compare pairs of indices. """ - # Regular grids dont really know about ring and position. We can try to see if - # their parent does! + # Regular grids don't know about ring and position. Check the parent. if ( self.armiObject is not None and self.armiObject.parent is not None @@ -1063,9 +1062,7 @@ def getRingPos(self, indices) -> Tuple[int, int]: ): return self.armiObject.parent.spatialGrid.getRingPos(indices) - # For compatibility's sake, return __something__. TODO: We may want to just - # throw here, to be honest. - return tuple(i + 1 for i in indices[:2]) + raise ValueError("No ring position found, because no spatial grid was found.") def getAllIndices(self): """Get all possible indices in this grid.""" diff --git a/armi/reactor/tests/test_grids.py b/armi/reactor/tests/test_grids.py index 79f0543e8..a9b602ba5 100644 --- a/armi/reactor/tests/test_grids.py +++ b/armi/reactor/tests/test_grids.py @@ -188,6 +188,14 @@ def test_getitem(self): self.assertIsInstance(multiLoc, grids.MultiIndexLocation) self.assertIn((1, 0, 0), grid._locations) + def test_ringPosFromIndicesIncorrect(self): + """Test the getRingPos fails if there is no armiObect or parent.""" + grid = grids.Grid(unitSteps=((1.0, 0.0, 0.0), (0.0, 1.0, 0.0), (0.0, 0.0, 1.0))) + + grid.armiObject = None + with self.assertRaises(ValueError): + grid.getRingPos(((0, 0), (1, 1))) + class TestHexGrid(unittest.TestCase): """A set of tests for the Hexagonal Grid. diff --git a/armi/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py index 16a126ffb..89e909e8e 100644 --- a/armi/settings/fwSettings/globalSettings.py +++ b/armi/settings/fwSettings/globalSettings.py @@ -17,9 +17,6 @@ This should contain Settings definitions for general-purpose "framework" settings. These should only include settings that are not related to any particular physics or plugins. - -TODO: There are lots of settings in here that violate the above rule, which still need -to be migrated to their respective plugins: they are clearly separated for review. """ import os from typing import List From 48f543859e84428a3edbae5bc0b0b38b7f3110af Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Thu, 17 Aug 2023 15:47:36 -0700 Subject: [PATCH 15/17] Removing unused settings (#1393) I removed the settings: * CONF_CONDITIONAL_MODULE_NAME * CONF_MEM_PER_NODE * CONF_NUM_CONTROL_BLOCKS * CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD Two of the settings marked under the TODO before I found were actually used in multiple downstream repos. so I determined to leave them in ARMI. --- armi/settings/fwSettings/globalSettings.py | 38 ++-------------------- armi/tests/test_apps.py | 1 - 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/armi/settings/fwSettings/globalSettings.py b/armi/settings/fwSettings/globalSettings.py index 89e909e8e..3bf5cfb89 100644 --- a/armi/settings/fwSettings/globalSettings.py +++ b/armi/settings/fwSettings/globalSettings.py @@ -71,6 +71,7 @@ CONF_FLUX_RECON = "fluxRecon" # strange coupling in fuel handlers CONF_FRESH_FEED_TYPE = "freshFeedType" CONF_GEOM_FILE = "geomFile" +CONF_GROW_TO_FULL_CORE_AFTER_LOAD = "growToFullCoreAfterLoad" CONF_INDEPENDENT_VARIABLES = "independentVariables" CONF_INITIALIZE_BURN_CHAIN = "initializeBurnChain" CONF_INPUT_HEIGHTS_HOT = "inputHeightsConsideredHot" @@ -93,6 +94,7 @@ CONF_POWER_FRACTIONS = "powerFractions" CONF_PROFILE = "profile" CONF_REALLY_SMALL_RUN = "reallySmallRun" +CONF_REMOVE_PER_CYCLE = "removePerCycle" CONF_RUN_TYPE = "runType" CONF_SKIP_CYCLES = "skipCycles" CONF_SMALL_RUN = "smallRun" @@ -114,14 +116,6 @@ CONF_VERSIONS = "versions" CONF_ZONE_DEFINITIONS = "zoneDefinitions" -# TODO: Unused by ARMI, slated for removal -CONF_CONDITIONAL_MODULE_NAME = "conditionalModuleName" # mcfr -CONF_GROW_TO_FULL_CORE_AFTER_LOAD = "growToFullCoreAfterLoad" # mcnp & gui -CONF_MEM_PER_NODE = "memPerNode" # unused -CONF_NUM_CONTROL_BLOCKS = "numControlBlocks" # unused -CONF_REMOVE_PER_CYCLE = "removePerCycle" # crucible, equilibrium, gui -CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD = "useInputTemperaturesOnDBLoad" # unused - def defineSettings() -> List[setting.Setting]: """Return a list of global framework settings.""" @@ -197,14 +191,6 @@ def defineSettings() -> List[setting.Setting]: "If false, block heights are at cold/as-built dimensions and will be thermally expanded as appropriate." ), ), - setting.Setting( - CONF_CONDITIONAL_MODULE_NAME, - default="", - label="Burn End Conditional", - description="File name (directory not included) of the Python " - "module that contains a conditional function to determine the end of burn " - "cycles", - ), setting.Setting( CONF_AUTOMATIC_VARIABLE_MESH, default=False, @@ -571,12 +557,6 @@ def defineSettings() -> List[setting.Setting]: description="Description needed", schema=vol.All(vol.Coerce(float), vol.Range(min=0, max=1)), ), - setting.Setting( - CONF_MEM_PER_NODE, - default=2000, - label="Memory per Node", - description="Memory requested per cluster node", - ), setting.Setting( CONF_MPI_TASKS_PER_NODE, default=0, @@ -596,12 +576,6 @@ def defineSettings() -> List[setting.Setting]: "cycles to be run after `startCycle`.", schema=vol.All(vol.Coerce(int), vol.Range(min=1)), ), - setting.Setting( - CONF_NUM_CONTROL_BLOCKS, - default=6, - label="Number of Control Blocks", - description="Number of blocks with control for a REBUS poison search", - ), setting.Setting( CONF_TIGHT_COUPLING, default=False, @@ -777,14 +751,6 @@ def defineSettings() -> List[setting.Setting]: description="The outlet temperature of the reactor in C", schema=vol.All(vol.Coerce(float), vol.Range(min=-273.15)), ), - setting.Setting( - CONF_USE_INPUT_TEMPERATURES_ON_DBLOAD, - default=False, - label="Temperatures From Input on DB Load", - description="When loading from a database, first set all component " - "temperatures to the input temperatures. Required when a coupled TH " - "case is being derived from a case without any coupled TH.", - ), setting.Setting( CONF_DEFERRED_INTERFACES_CYCLE, default=0, diff --git a/armi/tests/test_apps.py b/armi/tests/test_apps.py index a353b1ab3..6b534b545 100644 --- a/armi/tests/test_apps.py +++ b/armi/tests/test_apps.py @@ -24,7 +24,6 @@ from armi import isStableReleaseVersion from armi import meta from armi import plugins -from armi import utils from armi.reactor.flags import Flags from armi.__main__ import main From afabda96dd8b6022549555c124a523bb51d9cfc0 Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Tue, 22 Aug 2023 13:27:33 -0700 Subject: [PATCH 16/17] Ensuring we throw an error on bad YAML files (#1358) The goal is to throw an error when there is a duplicate key in the YAML file. Though if the key is heavily nested, deep in the YAML file, sometimes our YAML libraries will miss it. --- armi/nucDirectory/nuclideBases.py | 1 + .../blueprints/tests/test_blueprints.py | 19 +++++++++++++++++++ armi/reactor/systemLayoutInput.py | 1 + armi/settings/caseSettings.py | 5 +++-- armi/settings/settingsIO.py | 1 + armi/settings/tests/test_settingsIO.py | 15 +++++++++++++++ 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/armi/nucDirectory/nuclideBases.py b/armi/nucDirectory/nuclideBases.py index 3cc9427a3..6a2143ba5 100644 --- a/armi/nucDirectory/nuclideBases.py +++ b/armi/nucDirectory/nuclideBases.py @@ -1082,6 +1082,7 @@ def imposeBurnChain(burnChainStream): return burnChainImposed = True yaml = YAML(typ="rt") + yaml.allow_duplicate_keys = False burnData = yaml.load(burnChainStream) for nucName, burnInfo in burnData.items(): diff --git a/armi/reactor/blueprints/tests/test_blueprints.py b/armi/reactor/blueprints/tests/test_blueprints.py index 9428e2c21..81a54dfac 100644 --- a/armi/reactor/blueprints/tests/test_blueprints.py +++ b/armi/reactor/blueprints/tests/test_blueprints.py @@ -199,6 +199,25 @@ class TestBlueprintsSchema(unittest.TestCase): 2 2 2 2 2 """ + def test_noDuplicateKeysInYamlBlueprints(self): + """ + Prove that if you duplicate a section of a YAML blueprint file, + a hard error will be thrown. + """ + # loop through a few different sections, to test blueprints broadly + sections = ["blocks:", "components:", "component groups:"] + for sectionName in sections: + # modify blueprint YAML to duplicate this section + yamlString = str(self._yamlString) + i = yamlString.find(sectionName) + lenSection = yamlString[i:].find("\n\n") + section = yamlString[i : i + lenSection] + yamlString = yamlString[:i] + section + yamlString[i : i + lenSection] + + # validate that this is now an invalid YAML blueprint + with self.assertRaises(Exception): + _design = blueprints.Blueprints.load(yamlString) + def test_assemblyParameters(self): cs = settings.Settings() design = blueprints.Blueprints.load(self._yamlString) diff --git a/armi/reactor/systemLayoutInput.py b/armi/reactor/systemLayoutInput.py index 73cfd890b..cac411c7f 100644 --- a/armi/reactor/systemLayoutInput.py +++ b/armi/reactor/systemLayoutInput.py @@ -275,6 +275,7 @@ def _readYaml(self, stream): consistent inputs. """ yaml = YAML() + yaml.allow_duplicate_keys = False tree = yaml.load(stream) tree = INPUT_SCHEMA(tree) self.assemTypeByIndices.clear() diff --git a/armi/settings/caseSettings.py b/armi/settings/caseSettings.py index d5ab87fcc..d94f634a1 100644 --- a/armi/settings/caseSettings.py +++ b/armi/settings/caseSettings.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -r""" +""" This defines a Settings object that acts mostly like a dictionary. It is meant to be treated mostly like a singleton, where each custom ARMI object has access to it. It contains global user settings like the core @@ -22,7 +22,7 @@ A settings object can be saved as or loaded from an YAML file. The ARMI GUI is designed to create this settings file, which is then loaded by an ARMI process on the cluster. -A primary case settings is created as ``masterCs`` +A primary case settings is created as ``masterCs``. """ import io import logging @@ -401,6 +401,7 @@ def getSettingsSetByUser(self, fPath): # from the settings file to know which settings are user-defined with open(fPath, "r") as stream: yaml = YAML() + yaml.allow_duplicate_keys = False tree = yaml.load(stream) userSettings = tree[settingsIO.Roots.CUSTOM] diff --git a/armi/settings/settingsIO.py b/armi/settings/settingsIO.py index 9fa701beb..475a61352 100644 --- a/armi/settings/settingsIO.py +++ b/armi/settings/settingsIO.py @@ -208,6 +208,7 @@ def _readYaml(self, stream): from armi.settings.fwSettings.globalSettings import CONF_VERSIONS yaml = YAML(typ="rt") + yaml.allow_duplicate_keys = False tree = yaml.load(stream) if "settings" not in tree: raise InvalidSettingsFileError( diff --git a/armi/settings/tests/test_settingsIO.py b/armi/settings/tests/test_settingsIO.py index dfdc3ec3e..d9054db31 100644 --- a/armi/settings/tests/test_settingsIO.py +++ b/armi/settings/tests/test_settingsIO.py @@ -22,6 +22,7 @@ from armi.cli import entryPoint from armi.settings import setting from armi.settings import settingsIO +from armi.tests import TEST_ROOT from armi.utils import directoryChangers from armi.utils.customExceptions import ( InvalidSettingsFileError, @@ -68,6 +69,20 @@ def test_basicSettingsReader(self): self.assertFalse(getattr(reader, "filelessBP")) self.assertEqual(getattr(reader, "path"), "") + def test_readFromFile(self): + with directoryChangers.TemporaryDirectoryChanger(): + inPath = os.path.join(TEST_ROOT, "armiRun.yaml") + outPath = "test_readFromFile.yaml" + + txt = open(inPath, "r").read() + verb = "branchVerbosity:" + txt0, txt1 = txt.split(verb) + newTxt = f"{txt0}{verb} fake\n {verb}{txt1}" + open(outPath, "w").write(newTxt) + + with self.assertRaises(InvalidSettingsFileError): + settings.caseSettings.Settings(outPath) + class SettingsRenameTests(unittest.TestCase): testSettings = [ From da28372a240e80418139d1069317f546af9bcd8d Mon Sep 17 00:00:00 2001 From: John Stilley <1831479+john-science@users.noreply.github.com> Date: Tue, 22 Aug 2023 14:47:39 -0700 Subject: [PATCH 17/17] Move max assembly number out of global scope (#1383) --- MANIFEST.in | 2 +- armi/bookkeeping/db/__init__.py | 8 +- armi/bookkeeping/db/database3.py | 22 +--- armi/bookkeeping/db/tests/test_comparedb3.py | 4 +- armi/bookkeeping/db/tests/test_database3.py | 45 ++----- ...st-ref.txt => armiRun-A0032-aHist-ref.txt} | 2 +- armi/mpiActions.py | 3 - .../physics/fuelCycle/fuelHandlerInterface.py | 1 + armi/physics/fuelCycle/fuelHandlers.py | 2 +- .../fuelCycle/tests/test_fuelHandlers.py | 6 +- armi/reactor/assemblies.py | 112 ++++++++--------- armi/reactor/assemblyLists.py | 80 +++++++++++-- armi/reactor/blocks.py | 1 - armi/reactor/blueprints/__init__.py | 16 +-- armi/reactor/converters/geometryConverters.py | 13 +- .../converters/tests/test_uniformMesh.py | 5 +- armi/reactor/converters/uniformMesh.py | 12 +- armi/reactor/reactorParameters.py | 7 ++ armi/reactor/reactors.py | 113 +++++++++++++++--- armi/reactor/tests/test_assemblies.py | 8 -- armi/reactor/tests/test_reactors.py | 33 ++++- armi/tests/armiRun-SHUFFLES.txt | 4 +- doc/release/0.2.rst | 1 + ruff.toml | 2 + 24 files changed, 297 insertions(+), 205 deletions(-) rename armi/bookkeeping/tests/{armiRun-A0039-aHist-ref.txt => armiRun-A0032-aHist-ref.txt} (98%) diff --git a/MANIFEST.in b/MANIFEST.in index 25454e9b0..372ebd74e 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,4 +1,4 @@ -include armi/bookkeeping/tests/armiRun-A0039-aHist-ref.txt +include armi/bookkeeping/tests/armiRun-A0032-aHist-ref.txt include armi/nuclearDataIO/cccc/tests/fixtures/labels.ascii include armi/nuclearDataIO/cccc/tests/fixtures/labels.binary include armi/nuclearDataIO/cccc/tests/fixtures/mc2v3.dlayxs diff --git a/armi/bookkeeping/db/__init__.py b/armi/bookkeeping/db/__init__.py index 213485398..6a1d69cdd 100644 --- a/armi/bookkeeping/db/__init__.py +++ b/armi/bookkeeping/db/__init__.py @@ -67,7 +67,7 @@ # re-export package components for easier import from .permissions import Permissions -from .database3 import Database3, updateGlobalAssemblyNum +from .database3 import Database3 from .databaseInterface import DatabaseInterface from .compareDB3 import compareDatabases from .factory import databaseFactory @@ -144,12 +144,6 @@ def loadOperator(pathToDb, loadCycle, loadNode, allowMissing=False): settings.setMasterCs(cs) - # Update the global assembly number because, if the user is loading a reactor from - # blueprints and does not have access to an operator, it is unlikely that there is - # another reactor that has alter the global assem num. Fresh cases typically want - # this updated. - updateGlobalAssemblyNum(r) - o = thisCase.initializeOperator(r=r) runLog.important( "The operator will not be in the same state that it was at that cycle and " diff --git a/armi/bookkeeping/db/database3.py b/armi/bookkeeping/db/database3.py index 30dc69cae..5f8cc3bfc 100644 --- a/armi/bookkeeping/db/database3.py +++ b/armi/bookkeeping/db/database3.py @@ -70,7 +70,6 @@ from armi.bookkeeping.db.typedefs import History, Histories from armi.nucDirectory import nuclideBases from armi.physics.neutronics.settings import CONF_LOADING_FILE -from armi.reactor import assemblies from armi.reactor import grids from armi.reactor import parameters from armi.reactor import systemLayoutInput @@ -101,18 +100,6 @@ def getH5GroupName(cycle: int, timeNode: int, statePointName: str = None) -> str return "c{:0>2}n{:0>2}{}".format(cycle, timeNode, statePointName or "") -def updateGlobalAssemblyNum(r) -> None: - """ - Updated the global assembly number counter in ARMI, using the assemblies - read from a database. - """ - assemNum = r.core.p.maxAssemNum - if assemNum is not None: - assemblies.setAssemNumCounter(int(assemNum + 1)) - else: - raise ValueError("Could not load maxAssemNum from the database") - - class Database3: """ Version 3 of the ARMI Database, handling serialization and loading of Reactor states. @@ -691,8 +678,7 @@ def load( Whether to emit a warning, rather than crash if reading a database with undefined parameters. Default False. updateGlobalAssemNum : bool, optional - Whether to update the global assembly number to the value stored in - r.core.p.maxAssemNum. Default True. + DeprecationWarning: This is unused. updateMasterCs : bool, optional Whether to apply the cs (whether provided as an argument or read from the database) as the primary for the case. Default True. Can be useful @@ -741,9 +727,11 @@ def load( ) root = comps[0][0] - # ensure the max assembly number is correct, unless the user says no if updateGlobalAssemNum: - updateGlobalAssemblyNum(root) + runLog.warning( + "The method input `updateGlobalAssemNum` is no longer used.", + single=True, + ) # return a Reactor object if cs[CONF_SORT_REACTOR]: diff --git a/armi/bookkeeping/db/tests/test_comparedb3.py b/armi/bookkeeping/db/tests/test_comparedb3.py index b195b566a..4c86c6706 100644 --- a/armi/bookkeeping/db/tests/test_comparedb3.py +++ b/armi/bookkeeping/db/tests/test_comparedb3.py @@ -125,7 +125,7 @@ def test_compareDatabaseDuplicate(self): self.assertEqual(diffs.nDiffs(), 0) def test_compareDatabaseSim(self): - """end-to-end test of compareDatabases() on very simlar databases.""" + """End-to-end test of compareDatabases() on very simlar databases.""" # build two super-simple H5 files for testing o, r = test_reactors.loadTestReactor( TEST_ROOT, customSettings={"reloadDBName": "reloadingDB.h5"} @@ -177,7 +177,7 @@ def test_compareDatabaseSim(self): dbs[1]._fullPath, timestepCompare=[(0, 0), (0, 1)], ) - self.assertEqual(len(diffs.diffs), 465) + self.assertEqual(len(diffs.diffs), 468) # Cycle length is only diff (x3) self.assertEqual(diffs.nDiffs(), 3) diff --git a/armi/bookkeeping/db/tests/test_database3.py b/armi/bookkeeping/db/tests/test_database3.py index b72252551..3cc72635e 100644 --- a/armi/bookkeeping/db/tests/test_database3.py +++ b/armi/bookkeeping/db/tests/test_database3.py @@ -81,7 +81,14 @@ def test_writeToDB(self): self.assertEqual(sorted(self.db.h5db["c00n00"].keys()), sorted(keys)) # validate availabilityFactor did not make it into the H5 file - rKeys = ["cycle", "cycleLength", "flags", "serialNum", "timeNode"] + rKeys = [ + "maxAssemNum", + "cycle", + "cycleLength", + "flags", + "serialNum", + "timeNode", + ] self.assertEqual( sorted(self.db.h5db["c00n00"]["Reactor"].keys()), sorted(rKeys) ) @@ -109,8 +116,7 @@ def makeShuffleHistory(self): # Serial numbers *are not stable* (i.e., they can be different between test runs # due to parallelism and test run order). However, they are the simplest way to # check correctness of location-based history tracking. So we stash the serial - # numbers at the location of interest so that we can use them later to check our - # work. + # numbers at the location of interest so we can use them later to check our work. self.centralAssemSerialNums = [] self.centralTopBlockSerialNums = [] @@ -365,39 +371,6 @@ def test_loadSortSetting(self): self.assertEqual(len(r0), len(r1)) self.assertEqual(len(r0.core), len(r1.core)) - def test_load_updateGlobalAssemNum(self): - from armi.reactor import assemblies - from armi.reactor.assemblies import resetAssemNumCounter - - self.makeHistory() - - resetAssemNumCounter() - self.assertEqual(assemblies._assemNum, 0) - - r = self.db.load(0, 0, allowMissing=True, updateGlobalAssemNum=False) - # len(r.sfp) is zero but these nums are still reserved - numSFPBlueprints = 4 - expectedNum = len(r.core) + numSFPBlueprints - self.assertEqual(assemblies._assemNum, expectedNum) - - # now do the same call again and show that the global _assemNum keeps going up. - # in db.load, rector objects are built in layout._initComps() so the global assem num - # will continue to grow (in this case, double). - self.db.load(0, 0, allowMissing=True, updateGlobalAssemNum=False) - self.assertEqual(assemblies._assemNum, expectedNum * 2) - - # now load but set updateGlobalAssemNum=True and show that the global assem num - # is updated and equal to self.r.p.maxAssemNum + 1 which is equal to the number of - # assemblies in blueprints/core. - r = self.db.load(0, 0, allowMissing=True, updateGlobalAssemNum=True) - expected = len(self.r.core) + len(self.r.blueprints.assemblies.values()) - self.assertEqual(15, expected) - - # repeat the test above to show that subsequent db loads (with updateGlobalAssemNum=True) - # do not continue to increase the global assem num. - self.db.load(0, 0, allowMissing=True, updateGlobalAssemNum=True) - self.assertEqual(15, expected) - def test_history(self): self.makeShuffleHistory() diff --git a/armi/bookkeeping/tests/armiRun-A0039-aHist-ref.txt b/armi/bookkeeping/tests/armiRun-A0032-aHist-ref.txt similarity index 98% rename from armi/bookkeeping/tests/armiRun-A0039-aHist-ref.txt rename to armi/bookkeeping/tests/armiRun-A0032-aHist-ref.txt index d08e9d04a..c8911acb8 100644 --- a/armi/bookkeeping/tests/armiRun-A0039-aHist-ref.txt +++ b/armi/bookkeeping/tests/armiRun-A0032-aHist-ref.txt @@ -47,7 +47,7 @@ EOL bottom top center Assembly info -A0039 outer core fuel +A0032 outer core fuel "reflector" C A "fuel" C A "fuel" C A diff --git a/armi/mpiActions.py b/armi/mpiActions.py index e9c12e404..383adf9aa 100644 --- a/armi/mpiActions.py +++ b/armi/mpiActions.py @@ -68,7 +68,6 @@ from armi import settings from armi import utils from armi.reactor import reactors -from armi.reactor import assemblies from armi.reactor.parameters import parameterDefinitions from armi.utils import iterables @@ -610,8 +609,6 @@ def _distributeReactor(self, cs): runLog.debug( "The reactor has {} assemblies".format(len(self.r.core.getAssemblies())) ) - numAssemblies = self.broadcast(assemblies.getAssemNum()) - assemblies.setAssemNumCounter(numAssemblies) # attach here so any interface actions use a properly-setup reactor. self.o.reattach(self.r, cs) # sets r and cs diff --git a/armi/physics/fuelCycle/fuelHandlerInterface.py b/armi/physics/fuelCycle/fuelHandlerInterface.py index 4eccd3e64..dbaf5db37 100644 --- a/armi/physics/fuelCycle/fuelHandlerInterface.py +++ b/armi/physics/fuelCycle/fuelHandlerInterface.py @@ -105,6 +105,7 @@ def manageFuel(self, cycle): self.r.core.locateAllAssemblies() shuffleFactors, _ = fh.getFactorList(cycle) fh.outage(shuffleFactors) # move the assemblies around + if self.cs[CONF_PLOT_SHUFFLE_ARROWS]: arrows = fh.makeShuffleArrows() plotting.plotFaceMap( diff --git a/armi/physics/fuelCycle/fuelHandlers.py b/armi/physics/fuelCycle/fuelHandlers.py index 67758029f..1d51ae223 100644 --- a/armi/physics/fuelCycle/fuelHandlers.py +++ b/armi/physics/fuelCycle/fuelHandlers.py @@ -90,7 +90,7 @@ def r(self): return self.o.r def outage(self, factor=1.0): - r""" + """ Simulates a reactor reload outage. Moves and tracks fuel. This sets the moveList structure. diff --git a/armi/physics/fuelCycle/tests/test_fuelHandlers.py b/armi/physics/fuelCycle/tests/test_fuelHandlers.py index cccf1b4aa..c4ebaa7d9 100644 --- a/armi/physics/fuelCycle/tests/test_fuelHandlers.py +++ b/armi/physics/fuelCycle/tests/test_fuelHandlers.py @@ -398,7 +398,7 @@ def test_repeatShuffles(self): for a in self.r.sfp.getChildren(): self.assertEqual(a.getLocation(), "SFP") - # do some shuffles. + # do some shuffles fh = self.r.o.getInterface("fuelHandler") self.runShuffling(fh) # changes caseTitle @@ -455,7 +455,7 @@ def test_readMoves(self): sfpMove = moves[2][-2] self.assertEqual(sfpMove[0], "SFP") self.assertEqual(sfpMove[1], "005-003") - self.assertEqual(sfpMove[4], "A0085") # name of assem in SFP + self.assertEqual(sfpMove[4], "A0077") # name of assem in SFP def test_processMoveList(self): fh = fuelHandlers.FuelHandler(self.o) @@ -468,7 +468,7 @@ def test_processMoveList(self): loadNames, _, ) = fh.processMoveList(moves[2]) - self.assertIn("A0085", loadNames) + self.assertIn("A0077", loadNames) self.assertIn(None, loadNames) self.assertNotIn("SFP", loadChains) self.assertNotIn("LoadQueue", loadChains) diff --git a/armi/reactor/assemblies.py b/armi/reactor/assemblies.py index da89dfd5e..a0dbc5a41 100644 --- a/armi/reactor/assemblies.py +++ b/armi/reactor/assemblies.py @@ -20,6 +20,7 @@ import copy import math import pickle +from random import randint import numpy from scipy import interpolate @@ -33,31 +34,6 @@ from armi.reactor.flags import Flags from armi.reactor.parameters import ParamLocation -# to count the blocks that we create and generate a block number -_assemNum = 0 - - -def incrementAssemNum(): - global _assemNum # tracked on a module level - val = _assemNum # return value before incrementing. - _assemNum += 1 - return val - - -def getAssemNum(): - global _assemNum - return _assemNum - - -def resetAssemNumCounter(): - setAssemNumCounter(0) - - -def setAssemNumCounter(val): - runLog.extra("Resetting global assembly number to {0}".format(val)) - global _assemNum - _assemNum = val - class Assembly(composites.Composite): """ @@ -80,8 +56,7 @@ class Assembly(composites.Composite): SPENT_FUEL_POOL = "SFP" # For assemblies coming in from the database, waiting to be loaded to their old # position. This is a necessary distinction, since we need to make sure that a bunch - # of fuel management stuff doesn't treat its re-placement into the core as a new - # move + # of fuel management stuff doesn't treat its re-placement into the core as a new move DATABASE = "database" NOT_IN_CORE = [LOAD_QUEUE, SPENT_FUEL_POOL] @@ -93,11 +68,13 @@ def __init__(self, typ, assemNum=None): Name of assembly design (e.g. the name from the blueprints input file). assemNum : int, optional - The unique ID number of this assembly. If none is passed, the class-level - value will be taken and then incremented. + The unique ID number of this assembly. If None is provided, we generate a + random int. This makes it clear that it is a placeholder. When an assembly with + a negative ID is placed into a Reactor, it will be given a new, positive ID. """ + # If no assembly number is provided, generate a random number as a placeholder. if assemNum is None: - assemNum = incrementAssemNum() + assemNum = randint(-9e12, -1) name = self.makeNameFromAssemNum(assemNum) composites.Composite.__init__(self, name) self.p.assemNum = assemNum @@ -137,18 +114,24 @@ def __lt__(self, other): except ValueError: return False - def makeUnique(self): + def renameBlocksAccordingToAssemblyNum(self): """ - Function to make an assembly unique by getting a new assembly number. + Updates the names of all blocks to comply with the assembly number. - This also adjusts the assembly's blocks IDs. This is necessary when using - ``deepcopy`` to get a unique ``assemNum`` since a deepcopy implies it would - otherwise have been the same object. + Useful after an assembly number/name has been loaded from a snapshot and you + want to update all block names to be consistent. + + It may be better to store block numbers on each block as params. A database that + can hold strings would be even better. + + Notes + ----- + You must run armi.reactor.reactors.Reactor.regenAssemblyLists after calling + this. """ - self.p.assemNum = incrementAssemNum() - self.name = self.makeNameFromAssemNum(self.p.assemNum) + assemNum = self.getNum() for bi, b in enumerate(self): - b.setName(b.makeName(self.p.assemNum, bi)) + b.setName(b.makeName(assemNum, bi)) @staticmethod def makeNameFromAssemNum(assemNum): @@ -157,8 +140,35 @@ def makeNameFromAssemNum(assemNum): AssemNums are like serial numbers for assemblies. """ - name = "A{0:04d}".format(int(assemNum)) - return name + return "A{0:04d}".format(int(assemNum)) + + def renumber(self, newNum): + """ + Change the assembly number of this assembly. + + And handle the downstream impacts of changing the name of this Assembly and all + of the Blocks within this Assembly. + + Parameters + ---------- + newNum : int + The new Assembly number. + """ + self.p.assemNum = int(newNum) + self.name = self.makeNameFromAssemNum(self.p.assemNum) + self.renameBlocksAccordingToAssemblyNum() + + def makeUnique(self): + """ + Function to make an assembly unique by getting a new assembly number. + + This also adjusts the assembly's blocks IDs. This is necessary when using + ``deepcopy`` to get a unique ``assemNum`` since a deepcopy implies it would + otherwise have been the same object. + """ + # Default to a random negative assembly number (unique enough) + self.p.assemNum = randint(-9e12, -1) + self.renumber(self.p.assemNum) def add(self, obj): """ @@ -169,8 +179,7 @@ def add(self, obj): """ composites.Composite.add(self, obj) obj.spatialLocator = self.spatialGrid[0, 0, len(self) - 1] - # assemblies have bounds-based 1-D spatial grids. Adjust it to have the right - # value. + # assemblies have bounds-based 1-D spatial grids. Adjust it to the right value. if len(self.spatialGrid._bounds[2]) < len(self): self.spatialGrid._bounds[2][len(self)] = ( self.spatialGrid._bounds[2][len(self) - 1] + obj.getHeight() @@ -1132,25 +1141,6 @@ def reestablishBlockOrder(self): # update the name too. NOTE: You must update the history tracker. b.setName(b.makeName(self.p.assemNum, zi)) - def renameBlocksAccordingToAssemblyNum(self): - """ - Updates the names of all blocks to comply with the assembly number. - - Useful after an assembly number/name has been loaded from a snapshot and you - want to update all block names to be consistent. - - It may be better to store block numbers on each block as params. A database that - can hold strings would be even better. - - Notes - ----- - You must run armi.reactor.reactors.Reactor.regenAssemblyLists after calling - this. - """ - assemNum = self.getNum() - for bi, b in enumerate(self): - b.setName(b.makeName(assemNum, bi)) - def countBlocksWithFlags(self, blockTypeSpec=None): """ Returns the number of blocks of a specified type. diff --git a/armi/reactor/assemblyLists.py b/armi/reactor/assemblyLists.py index 71ba63ed3..97aa7259e 100644 --- a/armi/reactor/assemblyLists.py +++ b/armi/reactor/assemblyLists.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -r""" +""" Module containing :py:class:`AssemblyList` and related classes. Assembly Lists are core-like objects that store collections of Assemblies. They were @@ -21,7 +21,8 @@ Presently, the :py:class:`armi.reactor.reactors.Core` constructs a spent fuel pool `self.sfp`. We are in the process of removing these as instance attributes of the -``Core``, and moving them into sibling systems on the root :py:class:`armi.reactor.reactors.Reactor` object. +``Core``, and moving them into sibling systems on the root +:py:class:`armi.reactor.reactors.Reactor` object. """ import abc import itertools @@ -129,15 +130,12 @@ def add(self, assem, loc=None): The Assembly to add to the list loc : LocationBase, optional - If provided, the assembly is inserted at that location, similarly to how a - Core would function. If it is not provided, the locator on the Assembly - object will be used. If the Assembly's locator belongs to - ``self.spatialGrid``, the Assembly's existing locator will not be used. - This is unlike the Core, which would try to use the same indices, but move - the locator to the Core's grid. If no locator is passed, or if the - Assembly's locator is not in the AssemblyList's grid, then the Assembly will - be automatically located in the grid using the associated ``AutoFiller`` - object. + If provided, the assembly is inserted at that location. If it is not + provided, the locator on the Assembly object will be used. If the + Assembly's locator belongs to ``self.spatialGrid``, the Assembly's + existing locator will not be used. This is unlike the Core, which would try + to use the same indices, but move the locator to the Core's grid. With a + locator, the associated ``AutoFiller`` will be used. """ if loc is not None and loc.grid is not self.spatialGrid: raise ValueError( @@ -170,6 +168,7 @@ def getAssembly(self, name): def count(self): if not self.getChildren(): return + runLog.important("Count:") totCount = 0 thisTimeCount = 0 @@ -194,6 +193,31 @@ def count(self): class SpentFuelPool(AssemblyList): """A place to put assemblies when they've been discharged. Can tell you inventory stats, etc.""" + def add(self, assem, loc=None): + """ + Add an Assembly to the list. + + Parameters + ---------- + assem : Assembly + The Assembly to add to the list + + loc : LocationBase, optional + If provided, the assembly is inserted at that location. If it is not + provided, the locator on the Assembly object will be used. If the + Assembly's locator belongs to ``self.spatialGrid``, the Assembly's + existing locator will not be used. This is unlike the Core, which would try + to use the same indices, but move the locator to the Core's grid. With a + locator, the associated ``AutoFiller`` will be used. + """ + # If the assembly added has a negative ID, that is a placeholder, fix it. + if assem.p.assemNum < 0: + # update the assembly count in the Reactor + newNum = self.r.incrementAssemNum() + assem.renumber(newNum) + + super().add(assem, loc) + def report(self): title = "{0} Report".format(self.name) runLog.important("-" * len(title)) @@ -219,3 +243,37 @@ def report(self): self, totFis / 1000.0 ) ) + + def normalizeNames(self, startIndex=0): + """ + Renumber and rename all the Assemblies and Blocks. + + Parameters + ---------- + startIndex : int, optional + The default is to start counting at zero. But if you are renumbering assemblies + across the entire Reactor, you may want to start at a different number. + + Returns + ------- + int + The new max Assembly number. + """ + ind = startIndex + for a in self.getChildren(): + oldName = a.getName() + newName = a.makeNameFromAssemNum(ind) + if oldName == newName: + ind += 1 + continue + + a.p.assemNum = ind + a.setName(newName) + + for b in a: + axialIndex = int(b.name.split("-")[-1]) + b.name = b.makeName(ind, axialIndex) + + ind += 1 + + return int diff --git a/armi/reactor/blocks.py b/armi/reactor/blocks.py index 1b2322766..40f90b919 100644 --- a/armi/reactor/blocks.py +++ b/armi/reactor/blocks.py @@ -266,7 +266,6 @@ def getSmearDensity(self, cold=True): ------- smearDensity : float The smear density as a fraction - """ fuels = self.getComponents(Flags.FUEL) if not fuels: diff --git a/armi/reactor/blueprints/__init__.py b/armi/reactor/blueprints/__init__.py index 0197126ac..923e6a94b 100644 --- a/armi/reactor/blueprints/__init__.py +++ b/armi/reactor/blueprints/__init__.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -r""" +""" Blueprints describe the geometric and composition details of the objects in the reactor (e.g. fuel assemblies, control rods, etc.). @@ -305,23 +305,11 @@ def _prepConstruction(self, cs): self._assembliesBySpecifier.clear() self.assemblies.clear() - # retrieve current count of assemblies to restore after - # creating blueprints assemblies. This is particularly useful for - # doing snapshot based runs and multiple database loads, and ensures - # that each database load/snapshot to not cumulative increase the assembly - # count during creation of blueprints assemblies. During initial - # constructions the first N numbers are reserved for blueprints, so this - # ensures consistency. - currentCount = assemblies.getAssemNum() - # reset the assembly counter so that blueprints assemblies are always - # numbered 0 to len(self.assemDesigns) - assemblies.resetAssemNumCounter() for aDesign in self.assemDesigns: a = aDesign.construct(cs, self) self._assembliesBySpecifier[aDesign.specifier] = a self.assemblies[aDesign.name] = a - if currentCount != 0: - assemblies.setAssemNumCounter(currentCount) + runLog.header("=========== Verifying Assembly Configurations ===========") self._checkAssemblyAreaConsistency(cs) diff --git a/armi/reactor/converters/geometryConverters.py b/armi/reactor/converters/geometryConverters.py index 75ae2b003..6c05a5269 100644 --- a/armi/reactor/converters/geometryConverters.py +++ b/armi/reactor/converters/geometryConverters.py @@ -96,10 +96,6 @@ def reset(self): runLog.info( f"Resetting the state of the converted reactor core model in {self}" ) - currentAssemCounter = assemblies.getAssemNum() - assemblies.setAssemNumCounter( - currentAssemCounter - len(self._newAssembliesAdded) - ) self._newAssembliesAdded = [] @@ -253,7 +249,7 @@ def convert(self, r): ) def addRing(self, assemType="big shield"): - r""" + """ Add a ring of fuel assemblies around the outside of an existing core. Works by first finding the assembly furthest from the center, then filling in @@ -1428,8 +1424,8 @@ def addEdgeAssemblies(self, core): ) def removeEdgeAssemblies(self, core): - r""" - remove the edge assemblies in preparation for the nodal diffusion approximation. + """ + Remove the edge assemblies in preparation for the nodal diffusion approximation. This makes use of the assemblies knowledge of if it is in a region that it needs to be removed. @@ -1464,7 +1460,8 @@ def removeEdgeAssemblies(self, core): pDefs = parameters.ALL_DEFINITIONS.unchanged_since(NEVER) pDefs.setAssignmentFlag(SINCE_LAST_GEOMETRY_TRANSFORMATION) else: - runLog.extra("No edge assemblies to remove") + runLog.debug("No edge assemblies to remove.") + self.reset() @staticmethod diff --git a/armi/reactor/converters/tests/test_uniformMesh.py b/armi/reactor/converters/tests/test_uniformMesh.py index fb1ad27a6..a5a0869bd 100644 --- a/armi/reactor/converters/tests/test_uniformMesh.py +++ b/armi/reactor/converters/tests/test_uniformMesh.py @@ -366,6 +366,8 @@ def applyNonUniformHeightDistribution(reactor): a[-1].setHeight(a[-1].getHeight() - delta) a.calculateZCoords() + reactor.normalizeNames() + class TestUniformMesh(unittest.TestCase): """ @@ -411,7 +413,7 @@ def test_convertNumberDensities(self): ) # conversion didn't change source reactor mass def test_applyStateToOriginal(self): - applyNonUniformHeightDistribution(self.r) # note: this perturbs the ref. mass + applyNonUniformHeightDistribution(self.r) # note: this perturbs the ref mass self.converter.convert(self.r) for ib, b in enumerate(self.converter.convReactor.core.getBlocks()): @@ -685,7 +687,6 @@ def test_reactorConversion(self): self.assertFalse(b.p.rateAbs) self.converter.convert(self.r) - self.assertEqual( len(controlAssems), len(self.converter._nonUniformAssemStorage), diff --git a/armi/reactor/converters/uniformMesh.py b/armi/reactor/converters/uniformMesh.py index a89349ace..c404b4b80 100644 --- a/armi/reactor/converters/uniformMesh.py +++ b/armi/reactor/converters/uniformMesh.py @@ -160,6 +160,7 @@ def _computeAverageAxialMesh(self): aMesh = src.core.findAllAxialMeshPoints([a])[1:] if len(aMesh) == refNumPoints: allMeshes.append(aMesh) + averageMesh = average1DWithinTolerance(numpy.array(allMeshes)) self._commonMesh = numpy.array(averageMesh) @@ -433,12 +434,12 @@ def convert(self, r=None): includePinCoordinates=self.includePinCoordinates, ) homogAssem.spatialLocator = assem.spatialLocator + homogAssem.p.assemNum = assem.p.assemNum - # Remove this assembly from the core and add it to the - # temporary storage list so that it can be replaced with the homogenized assembly. - # Note that we do not call the `removeAssembly` method because - # this will delete the core assembly from existence rather than - # only stripping its spatialLocator away. + # Remove this assembly from the core and add it to the temporary storage + # so that it can be replaced with the homogenized assembly. Note that we + # do not call `removeAssembly()` because this will delete the core + # assembly from existence rather than only stripping its spatialLocator. if assem.spatialLocator in self.convReactor.core.childrenByLocator: self.convReactor.core.childrenByLocator.pop(assem.spatialLocator) self.convReactor.core.remove(assem) @@ -449,7 +450,6 @@ def convert(self, r=None): assem.setName(assem.getName() + self._TEMP_STORAGE_NAME_SUFFIX) self._nonUniformAssemStorage.add(assem) self.convReactor.core.add(homogAssem) - else: runLog.extra(f"Building copy of {r} with a uniform axial mesh.") self.convReactor = self.initNewReactor(r, self._cs) diff --git a/armi/reactor/reactorParameters.py b/armi/reactor/reactorParameters.py index 2fe1bc5bc..c30979741 100644 --- a/armi/reactor/reactorParameters.py +++ b/armi/reactor/reactorParameters.py @@ -86,6 +86,13 @@ def defineReactorParameters(): "timeNode", units=units.UNITLESS, description="Integer timeNode", default=0 ) + pb.defParam( + "maxAssemNum", + units=units.UNITLESS, + description="Max number of assemblies created so far in the Reactor (integer)", + default=0, + ) + with pDefs.createBuilder( location=ParamLocation.AVERAGE, default=0.0, categories=["economics"] ) as pb: diff --git a/armi/reactor/reactors.py b/armi/reactor/reactors.py index 291fd8b3f..09f4ece19 100644 --- a/armi/reactor/reactors.py +++ b/armi/reactor/reactors.py @@ -40,7 +40,6 @@ from armi import getPluginManagerOrFail, materials, nuclearDataIO from armi import runLog from armi.nuclearDataIO import xsLibraries -from armi.reactor import assemblies from armi.reactor import assemblyLists from armi.reactor import composites from armi.reactor import geometry @@ -75,6 +74,7 @@ def __init__(self, name, blueprints): self.o = None self.spatialGrid = None self.spatialLocator = None + self.p.maxAssemNum = 0 self.p.cycle = 0 self.p.flags |= Flags.REACTOR self.core = None @@ -111,6 +111,47 @@ def add(self, container): ) self.core = cores[0] + def incrementAssemNum(self): + """ + Increase the max assembly number by one and returns the current value. + + Notes + ----- + The "max assembly number" is not currently used in the Reactor. So the idea + is that we return the current number, then iterate it for the next assembly. + + Obviously, this method will be unused for non-assembly-based reactors. + + Returns + ------- + int + The new max Assembly number. + """ + val = int(self.p.maxAssemNum) + self.p.maxAssemNum += 1 + return val + + def normalizeNames(self): + """ + Renumber and rename all the Assemblies and Blocks. + + This method normalizes the names in the Core then the SFP. + + Returns + ------- + int + The new max Assembly number. + """ + self.p.maxAssemNum = 0 + + ind = self.core.normalizeNames(self.p.maxAssemNum) + self.p.maxAssemNum = ind + + ind = self.sfp.normalizeNames(self.p.maxAssemNum) + self.p.maxAssemNum = ind + + return ind + def loadFromCs(cs) -> Reactor: """ @@ -154,6 +195,7 @@ def factory(cs, bp, geom: Optional[SystemLayoutInput] = None) -> Reactor: ) coreDesign = bp.systemDesigns["core"] coreDesign.construct(cs, bp, r, geom=geom) + for structure in bp.systemDesigns: if structure.name.lower() != "core": structure.construct(cs, bp, r) @@ -531,6 +573,52 @@ def removeAllAssemblies(self, discharge=True): self.parent.sfp.removeAll() self.blocksByName = {} self.assembliesByName = {} + self.parent.p.maxAssemNum = 0 + + def normalizeNames(self, startIndex=0): + """ + Renumber and rename all the Assemblies and Blocks. + + Parameters + ---------- + startIndex : int, optional + The default is to start counting at zero. But if you are renumbering assemblies + across the entire Reactor, you may want to start at a different number. + + Returns + ------- + int + The new max Assembly number. + """ + ind = startIndex + for a in self: + oldName = a.getName() + newName = a.makeNameFromAssemNum(ind) + if oldName == newName: + ind += 1 + continue + + a.p.assemNum = ind + a.setName(newName) + + for b in a: + axialIndex = int(b.name.split("-")[-1]) + b.name = b.makeName(ind, axialIndex) + + ind += 1 + + self.normalizeInternalBookeeping() + + return ind + + def normalizeInternalBookeeping(self): + """Update some bookkeeping dictionaries of assembly and block names in this Core.""" + self.assembliesByName = {} + self.blocksByName = {} + for assem in self: + self.assembliesByName[assem.getName()] = assem + for b in assem: + self.blocksByName[b.getName()] = b def add(self, a, spatialLocator=None): """ @@ -552,6 +640,10 @@ def add(self, a, spatialLocator=None): -------- removeAssembly : removes an assembly """ + # Negative assembly IDs are placeholders, and we need to renumber the assembly + if a.p.assemNum < 0: + a.renumber(self.r.incrementAssemNum()) + # resetting .assigned forces database to be rewritten for shuffled core paramDefs = set(parameters.ALL_DEFINITIONS) paramDefs.difference_update(set(parameters.forType(Core))) @@ -595,9 +687,7 @@ def add(self, a, spatialLocator=None): runLog.error( "The assembly {1} in the reactor already has the name {0}.\nCannot add {2}. " "Current assemNum is {3}" - "".format( - aName, self.assembliesByName[aName], a, assemblies.getAssemNum() - ) + "".format(aName, self.assembliesByName[aName], a, self.r.p.maxAssemNum) ) raise RuntimeError("Core already contains an assembly with the same name.") self.assembliesByName[aName] = a @@ -1269,7 +1359,6 @@ def getNuclideCategories(self): structureNuclides : set set of nuclide names - """ if not self._nuclideCategories: coolantNuclides = set() @@ -1485,8 +1574,8 @@ def getAssembly( """ if assemblyName: return self.getAssemblyByName(assemblyName) - for a in self.getAssemblies(*args, **kwargs): + for a in self.getAssemblies(*args, **kwargs): if a.getLocation() == locationString: return a if a.getNum() == assemNum: @@ -1507,8 +1596,6 @@ def getAssemblyWithAssemNum(self, assemNum): ------- foundAssembly : Assembly object or None The assembly found, or None - - """ return self.getAssembly(assemNum=assemNum) @@ -1532,7 +1619,6 @@ def getAssemblyPitch(self): ------- pitch : float The assembly pitch. - """ return self.spatialGrid.pitch @@ -1604,7 +1690,6 @@ def findNeighbors( See Also -------- grids.Grid.getSymmetricEquivalents - """ neighborIndices = self.spatialGrid.getNeighboringCellIndices( *a.spatialLocator.getCompleteIndices() @@ -1647,6 +1732,7 @@ def _getReflectiveDuplicateAssembly(self, neighborLoc): duplicateAssem = self.childrenByLocator.get(neighborLocation2) if duplicateAssem is not None: duplicates.append(duplicateAssem) + # should always be 0 or 1 nDuplicates = len(duplicates) if nDuplicates == 1: @@ -1946,7 +2032,7 @@ def addMoreNodes(self, meshList): return meshList, True def findAllAziMeshPoints(self, extraAssems=None, applySubMesh=True): - r""" + """ Returns a list of all azimuthal (theta)-mesh positions in the core. Parameters @@ -1957,7 +2043,6 @@ def findAllAziMeshPoints(self, extraAssems=None, applySubMesh=True): applySubMesh : bool generates submesh points to further discretize the theta reactor mesh - """ i, _, _ = self.findAllMeshPoints(extraAssems, applySubMesh) return i @@ -1966,7 +2051,6 @@ def findAllRadMeshPoints(self, extraAssems=None, applySubMesh=True): """ Return a list of all radial-mesh positions in the core. - Parameters ---------- extraAssems : list @@ -1976,7 +2060,6 @@ def findAllRadMeshPoints(self, extraAssems=None, applySubMesh=True): applySubMesh : bool (not implemented) generates submesh points to further discretize the radial reactor mesh - """ _, j, _ = self.findAllMeshPoints(extraAssems, applySubMesh) return j @@ -2004,7 +2087,7 @@ def getMaxNumPins(self): return max(b.getNumPins() for b in self.getBlocks()) def getMinimumPercentFluxInFuel(self, target=0.005): - r""" + """ Goes through the entire reactor to determine what percentage of flux occures at each ring. Starting with the outer ring, this function helps determine the effective size of the core where additional assemblies will not help the breeding in the TWR. diff --git a/armi/reactor/tests/test_assemblies.py b/armi/reactor/tests/test_assemblies.py index f8a93e460..21d916e2b 100644 --- a/armi/reactor/tests/test_assemblies.py +++ b/armi/reactor/tests/test_assemblies.py @@ -41,8 +41,6 @@ from armi.utils import directoryChangers from armi.utils import textProcessors from armi.reactor.tests import test_reactors -from armi.reactor.assemblies import getAssemNum -from armi.reactor.assemblies import resetAssemNumCounter from armi.physics.neutronics.settings import ( CONF_LOADING_FILE, CONF_XS_KERNEL, @@ -288,12 +286,6 @@ def test_notesParameter(self): self.assembly.p.notes = tooLongNote self.assertEqual(self.assembly.p.notes, tooLongNote[0:1000]) - def test_resetAssemNumCounter(self): - resetAssemNumCounter() - cur = 0 - ref = getAssemNum() - self.assertEqual(cur, ref) - def test_iter(self): cur = [] for block in self.assembly: diff --git a/armi/reactor/tests/test_reactors.py b/armi/reactor/tests/test_reactors.py index efcf9f211..eccc2428a 100644 --- a/armi/reactor/tests/test_reactors.py +++ b/armi/reactor/tests/test_reactors.py @@ -172,12 +172,10 @@ def loadTestReactor( fName = os.path.join(inputFilePath, inputFileName) customSettings = customSettings or {} isPickeledReactor = fName == ARMI_RUN_PATH and customSettings == {} - assemblies.resetAssemNumCounter() if isPickeledReactor and TEST_REACTOR: # return test reactor only if no custom settings are needed. o, r, assemNum = cPickle.loads(TEST_REACTOR) - assemblies.setAssemNumCounter(assemNum) settings.setMasterCs(o.cs) o.reattach(r, o.cs) return o, r @@ -210,7 +208,7 @@ def loadTestReactor( if isPickeledReactor: # cache it for fast load for other future tests # protocol=2 allows for classes with __slots__ but not __getstate__ to be pickled - TEST_REACTOR = cPickle.dumps((o, o.r, assemblies.getAssemNum()), protocol=2) + TEST_REACTOR = cPickle.dumps((o, o.r, o.r.p.maxAssemNum), protocol=2) return o, o.r @@ -321,7 +319,7 @@ def test_getBlocksByIndices(self): indices = [(1, 1, 1), (3, 2, 2)] actualBlocks = self.r.core.getBlocksByIndices(indices) actualNames = [b.getName() for b in actualBlocks] - expectedNames = ["B0022-001", "B0043-002"] + expectedNames = ["B0014-001", "B0035-002"] self.assertListEqual(expectedNames, actualNames) def test_getAllXsSuffixes(self): @@ -339,6 +337,28 @@ def test_countBlocksOfType(self): ) self.assertEqual(numControlBlocks, 3) + def test_normalizeNames(self): + # these are the correct, normalized names + numAssems = 73 + a = self.r.core.getFirstAssembly() + correctNames = [a.makeNameFromAssemNum(n) for n in range(numAssems)] + + # validate the reactor is what we think now + self.assertEqual(len(self.r.core), numAssems) + currentNames = [a.getName() for a in self.r.core] + self.assertNotEqual(correctNames, currentNames) + + # validate that we can normalize the names correctly once + self.r.normalizeNames() + currentNames = [a.getName() for a in self.r.core] + self.assertEqual(correctNames, currentNames) + + # validate that repeated applications of this method are stable + for _ in range(3): + self.r.normalizeNames() + currentNames = [a.getName() for a in self.r.core] + self.assertEqual(correctNames, currentNames) + def test_setB10VolOnCreation(self): """Test the setting of b.p.initialB10ComponentVol.""" for controlBlock in self.r.core.getBlocks(Flags.CONTROL): @@ -634,10 +654,11 @@ def test_getMinimumPercentFluxInFuel(self): def test_getAssembly(self): a1 = self.r.core.getAssemblyWithAssemNum(assemNum=10) - a2 = self.r.core.getAssembly(locationString="005-023") + a2 = self.r.core.getAssembly(locationString="003-001") a3 = self.r.core.getAssembly(assemblyName="A0010") - self.assertEqual(a1, a2) + self.assertEqual(a1, a3) + self.assertEqual(a1, a2) def test_restoreReactor(self): aListLength = len(self.r.core.getAssemblies()) diff --git a/armi/tests/armiRun-SHUFFLES.txt b/armi/tests/armiRun-SHUFFLES.txt index 7655f1260..e17307d19 100644 --- a/armi/tests/armiRun-SHUFFLES.txt +++ b/armi/tests/armiRun-SHUFFLES.txt @@ -13,7 +13,7 @@ Before cycle 2: 005-004 moved to 004-001 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 006-004 moved to 005-004 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 LoadQueue moved to 006-004 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 -SFP moved to 005-003 with assembly type feed fuel ANAME=A0085 with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 +SFP moved to 005-003 with assembly type feed fuel ANAME=A0077 with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 005-003 moved to SFP with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 Before cycle 3: @@ -23,6 +23,6 @@ Before cycle 3: 005-002 moved to 004-003 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 006-007 moved to 005-002 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 LoadQueue moved to 006-007 with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 -SFP moved to 005-004 with assembly type feed fuel ANAME=A0086 with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 +SFP moved to 005-004 with assembly type feed fuel ANAME=A0078 with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 005-004 moved to SFP with assembly type igniter fuel with enrich list: 0.00000000 0.11000000 0.11000000 0.11000000 0.00000000 diff --git a/doc/release/0.2.rst b/doc/release/0.2.rst index 44065b86c..a3026ffea 100644 --- a/doc/release/0.2.rst +++ b/doc/release/0.2.rst @@ -11,6 +11,7 @@ What's new in ARMI #. The Spent Fuel Pool (``sfp``) was moved from the ``Core`` out to the ``Reactor``. (`PR#1336 `_) #. Broad cleanup of ``Parameters``: filled in all empty units and descriptions, removed unused params. (`PR#1345 `_) #. Removed redundant ``Material.name`` variable. (`PR#1335 `_) +#. Moved the ``Reactor`` assembly number from the global scope to a ``Parameter``. (`PR#1383 `_) #. Added SHA1 hashes of XS control files to the welcome text. (`PR#1334 `_) #. Add python 3.11 to ARMI's CI testing GH actions! (`PR#1341 `_) #. Put back ``avgFuelTemp`` block parameter. (`PR#1362 `_) diff --git a/ruff.toml b/ruff.toml index 402d61c61..d852f3cf2 100644 --- a/ruff.toml +++ b/ruff.toml @@ -11,6 +11,8 @@ required-version = "0.0.272" # TID - tidy imports select = ["E", "F", "D", "N801", "SIM", "TID"] +# TODO: We want to support PLW0603 - don't use the global keyword + # Ruff rules we ignore (for now) because they are not 100% automatable # # D100 - Missing docstring in public module