Skip to content

Commit

Permalink
Merge remote-tracking branch 'terrapower/main' into drewj-usnctech/gr…
Browse files Browse the repository at this point in the history
…id-refactor-pre-1278

* terrapower/main:
  Move max assembly number out of global scope (terrapower#1383)
  Ensuring we throw an error on bad YAML files (terrapower#1358)
  Removing unused settings (terrapower#1393)
  Fixing some miscellaneous TODOs (terrapower#1392)
  Removing global plugin flags (terrapower#1388)
  Update reactivity coefficient parameters (terrapower#1355)
  Fixing ruff formatting of doc gallery examples (terrapower#1389)
  Fixing broken link (terrapower#1390)
  Removing unreachable code block (terrapower#1391)
  Removing unnecessary f-strings (terrapower#1387)
  Updating documentation dependencies for Python 3.11 (terrapower#1378)
  Adding a little code coverage to the CLIs (terrapower#1371)
  Remove coveragepy-lcov dependency during CI. (terrapower#1382)
  Reconnect logger after disconnecting in test. (terrapower#1381)
  Add Python 3.11 to GH Actions (terrapower#1341)
  Removing global variable from runLog.py (terrapower#1370)
  Moving the First-Time Contributors guide to the docs (terrapower#1368)
  • Loading branch information
drewj-usnctech committed Aug 24, 2023
2 parents a01054e + da28372 commit df4bfa4
Show file tree
Hide file tree
Showing 93 changed files with 717 additions and 516 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/black.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 .
Expand Down
16 changes: 2 additions & 14 deletions .github/workflows/coverage.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/[email protected]
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
path-to-lcov: lcov.txt

path-to-lcov: coverage.lcov
2 changes: 1 addition & 1 deletion .github/workflows/unittests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/validatemanifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/wintests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ doc/gallery
doc/gallery-src/framework/*.yaml
.coverage
coverage.xml
coverage.lcov
coverage_results.*
htmlcov/
monkeytype.*
Expand Down
78 changes: 2 additions & 76 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 1 addition & 1 deletion MANIFEST.in
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 8 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -58,7 +60,7 @@ found in [#touranarmi]_.

Quick start
-----------
Before starting, you need to have `Python <https://www.python.org/downloads/>`_ 3.7+ on
Before starting, you need to have `Python <https://www.python.org/downloads/>`_ 3.9+ on
Windows or Linux.

Get the ARMI code, install the prerequisites, and fire up the launcher with the following
Expand All @@ -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
<https://terrapower.github.io/armi/gallery/index.html>`_ and
`tutorials <https://terrapower.github.io/armi/tutorials/index.html>`_ to
Expand Down
2 changes: 1 addition & 1 deletion armi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions armi/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand Down
8 changes: 1 addition & 7 deletions armi/bookkeeping/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 "
Expand Down
22 changes: 5 additions & 17 deletions armi/bookkeeping/db/database3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down
1 change: 0 additions & 1 deletion armi/bookkeeping/db/databaseInterface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 2 additions & 9 deletions armi/bookkeeping/db/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions armi/bookkeeping/db/tests/test_comparedb3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down Expand Up @@ -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)

Expand Down
Loading

0 comments on commit df4bfa4

Please sign in to comment.