Global Parameters in ARMI

[opotowsky]

Global parameters cause issues. Mostly we run into said issues during unit testing, and global params prevent or run into issues with parallel unit test execution. Also, it is against the standards and practices of ARMI, so maybe it's high time we follow our own standards! We need to keep some of these, but let's review them all and see if there's anything we can improve. (I'm lookin at you, caseSettings and nuclideBases!)

1. Nuclides

• global elements ‎armi/nucDirectory/elements.py
• many global nuclide params armi/nucDirectory/nuclideBases.py
• !!! ‎armi/nucDirectory/thermalScattering.py This one actually doesn't get deleted during the above destroy function!!!!!

2. Case settings & Blueprints

• global Settings.instance ‎armi/settings/caseSettings.py (PR)
• global AssemNum armi/reactor/assemblies.py (PR)

3. Misc

• global white space ‎armi/runLog.py (PR)
• global plugin flags registered armi/reactor/flags.py (PR)
• global serial num armi/reactor/parameters/parameterCollections.py
• global test reactor ‎armi/reactor/tests/test_reactors.py (Hm, maybe this is the issue and not the case settings? haven't looked into this in awhile)

4. Prob want to keep?

• mpiActions -- but that's a whole 'nother thing.
• 4 global params ‎armi/context.py (and line 180)
• material namespace order ‎armi/materials/init.py

[john-science]

I totally agree. All (or maybe nearly all, we'll see) of these were just someone trying to save 10 minutes a few years ago, and now we're stuck with it.

Some of these are only bad ideas (looking at you _WHITE_SPACE in runLog.py). But some of these make running parallel ARMI cases significantly harder, or make having two ARMI reactors in memory at the same time impossible (looking at you case settings, blueprints, and plugins).

[john-science]

@drewj-usnctech Just FYI.

[drewj-usnctech]

Generally okay removing globals. And if it breaks something, we probably shouldn't have done it in the first place

[john-science]

Could we move parameterCollections.GLOBAL_SERIAL_NUM onto the Reactor or something?

[opotowsky]

Looks like it's only used in the database, that keeps track of its own layout.serialNum

[john-science]

Yeah, something like that:

armi/armi/bookkeeping/db/database3.py

Line 1107 in e8ddcdf

anchorSerialNum = anchor.p.serialNum

armi/armi/bookkeeping/db/database3.py

Lines 1276 to 1277 in e8ddcdf

	for c in comps:
	compsByTypeThenSerialNum[c.__class__][c.p.serialNum] = c

I just wonder if we can pull it out of the global Python space and put it into the Reactor (or maybe DB?) object somewhere. Just something so it is not global space. That way we could use this in MPI cases and we could allow for multiple reactors in memory at the same time.

[john-science]

Okay, I think it's clear the best option is to move the assemblies.py:_assemNum onto the Reactor object:

armi/armi/reactor/assemblies.py

Lines 36 to 44 in f947ea1

	# 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

And we can do the same for the parameterCollection.GLOBAL_SERIAL_NUM:

armi/armi/reactor/parameters/parameterCollections.py

Line 34 in e8ddcdf

GLOBAL_SERIAL_NUM = -1

The only question is:

Should these be attributes of the Reactor class, or Parameters?

Making them each a Parameter is slightly more work. But it would also mean they were stored in the database, which is probably nice/useful.

[opotowsky]

I would need someone else's input on if it would be a useful param. My knee-jerk reaction is that if it's not in the DB now, it doesn't need to be. In that case, I vote you make this a reactor attribute!

[albeanth]

many global nuclide params armi/nucDirectory/nuclideBases.py

This one reared its head for @wcscherer. Just bringing this convo back to life to maybe knock some of these out in the new year.

[opotowsky]

Adding a vote here to prioritize the global nuclides removal, as it has significant downstream impacts and prevents our unit tests from being parallelized.

[john-science]

@wcscherer @albeanth @opotowsky

I think that would be a solid improvement to the nucDir system. Help me build manager interest in this.

[jakehader]

@john-science - I'm in agreement of removing globals and moving in a direction where we can serialize any particular state of the system to the database, including enhancing the parameter system.

[john-science]

@jakehader I will try to convince management of WHY this is important, so I can put this work on the calendar for ARMI version 0.4.0.