Zones overhaul

[joshuavictorchen]

The armi.reactor.zones module is very SAS4A/SASSYS-1-specific and is in need of an overhaul - see this issue.

This discussion serves to communicate these requirements and manage progress/collaboration on implementation.

Note: In the following bullets we define the term Item to be either an Assembly or a Block. And probably, one Zone collection will have to be defined as all Assembly objects or all Block objects, with no mixing (just to make things easier).

Requirements

The following requirements describe what a good zones module should look like.

• zones should be an attribute of the Core that tracks Item locations AND the names of the Items in those locations
  • This means that Zones will be affected if any Items are moved, but the Items themselves can be perturbed without disturbing the Zones (i.e., updating their blocks’ reactivity coefficient parameters)
  • The primary function of zones will be to contain a list/set/collection of Zone objects.
• A Zone should have the following attributes:
  • name (string)
  • locations (dict of locations and corresponding Item names)
  • length (int of count of locations)
  • id (hash based on the locations attribute), or a hash() function
• Zones may be mutable, but their id attributes should be updated if any changes are made to them, in order to:
  • Validate that reactivity coefficient parameters were calculated for the same location-Item configuration as expected by the SAS model
  • Adjust Zones on the fly if needed (i.e., for sensitivity studies)
• Zones may have the following utility functions:
  • addItem(item, toZone)
  • transferItem(item, fromZone, toZone)
  • removeItem(item, fromZone)
  • checkDuplicates()
  • zoningStrategy() <-- abstract method?
• Users should be able to easily get specific Item objects described by the Zone
• Zones should be generated whenever a reactor database is created or loaded
  • Zones should be able to be written to/read from the database
• Zones should NOT be able to contain duplicate Items
• Zones should NOT need to be contiguous or contain contiguous Items
• Zones should NOT need to be comprehensive (i.e., zones don't have to contain ALL of the Items in the Core)

Implementation details:

• Unit tests should be written to ensure Zones
  • are easy to implement as a Plugin or Interface or something easy
  • can be saved to a DB
  • can be read from a DB

[joshuavictorchen]

Thanks to @john-science for helping to refine the above requirements.

The feature branch for this overhaul is: https://github.com/terrapower/armi/tree/zones_overhaul

[john-science]

Awesome!

Let's try and organize our work a bit. Just edit this comment and put your name next to the thing you're working on:

• Removing various current zoning strategies (JOHN)
  • Removing all references to Hot Zones (JOHN)
  • Removing all references to Ring Zones (JOHN)
• First try at flushing out the Zone and Zones classes. (JOHN)
• Changing how the zoningStrategy Setting works (JOHN)
  • Move the zoning strat to a Plugin hook.
  • Add Zoning Strategy example to the docs (JOHN)
• Refactor Zone Summary (JOHN)
  • Should we just print something simpler that you could copy/paste into manual zone definitions? (JOHN)
  • Remove duplicate Zone summary at: armi/bookkeeping/report/reportingUtils.py? (JOHN)
  • This thing ONLY works for Axial Reactors, and also not Blocks. (JOHN)
• Do we need to remove anything from the ARMI Docs? (NO)
• Implement zoning strategy for SAS/NATD use cases within the SAS Interface (JOSH)
• Make multiple Zoning Strategies raise and Error, rather than just a warning? (JOHN)
• Rename defineZoningStrategy() to applyZoningStrategy()? (JOHN)
• TBD...

[john-science]

@joshuavictorchen Okay, I have torn out a bunch of code. Mostly, I tore out all those old, strange "zoning strategies" that were applied by default. And I tore out some odd references to "Rings" that existed by default in the Zone class. (Not all Reactors might have rings, after all.)

So, we have a lot less cruft lying around in the zones.py class. But, of course, at some point, this will mean one or two downstream plugins will have to define their own Zoning Strats. But that's part of the Requirement.

[joshuavictorchen]

I'm thinking of kind of a hybrid scenario re: changing how the zoningStrategy Setting works - would it be possible to let the user select a specific plugin to implement their zoning strategy via a Setting?

Then that plugin's implementation of an abstract applyZoningStrategy class (or similar) would be called to create the Zones.

In my mind, this would also prevent plugins from fighting with/overriding each other's zones.

[john-science]

Okay, I was wondering how we were going to make the zoning strategies work. I had some vague ideas, but yours is better now that I look at it.

So, let me take a crack at this tonight/tomorrow morning. I looks like we would put a new abstractmethod here:

armi/armi/plugins.py

Line 141 in 9a510ca

class ArmiPlugin:

And add another abstractmethod / HOOK thing, like one of these:

armi/armi/plugins.py

Lines 345 to 349 in 9a510ca

	@staticmethod
	@HOOKSPEC
	def defineSettings():
	"""
	Define configuration settings for this plugin.

armi/armi/physics/fuelCycle/__init__.py

Lines 77 to 80 in 9a510ca

	@staticmethod
	@plugins.HOOKIMPL
	def defineSettings():
	return settings.getFuelCycleSettings()

And maybe call our new hook in buildZones():

armi/armi/reactor/zones.py

Line 392 in 9a510ca

def buildZones(core, cs):

In the same way the other hooks get called:

armi/armi/apps.py

Line 139 in 9a510ca

for pluginSettings in self._pm.hook.defineSettings():

And maybe we build a nice example for people, and put it in the docs. So everyone can see how to define their own zoning strategy in the future.

[john-science]

Minor Requirement Change

After looking at the code, I think we can simplify our design significantly if we allow a Zone to be a collection of either Assemblies or Blocks. The biggest reason for this is that both Assembly and Block have a getLocation() method. So, we could use those. The only difference we have to add one piece of data to the system: Each Zone would need to say with it is Assemblies or Blocks.

This would include where we read in Zone definitions. So, this isn't as simple as the old "Assembly-only" pathway, but it is worlds simpler than our original idea of allowing Component-level Zones.

[joshuavictorchen]

From my (selfish) Safety perspective, we just need the granularity to get down to the Assembly level - so I am 100% for this.

[john-science]

Also, @joshuavictorchen , I note that we already have logic in the code base for manually defining Zones in the Settings file:

armi/armi/settings/fwSettings/globalSettings.py

Lines 139 to 149 in e336bad

	setting.Setting(
	CONF_ZONING_STRATEGY,
	default="byRingZone",
	label="Automatic core zone creation strategy",
	description="Channel Grouping Options for Safety;"
	"everyFA: every FA is its own channel, "
	"byRingZone: based on ringzones, "
	"byFuelType: based on fuel type, "
	"Manual: you must specify 'zoneDefinitions' setting",
	options=["byRingZone", "byOrifice", "byFuelType", "everyFA", "manual"],
	),

And we even check for manual zone definitions:

armi/armi/operators/settingsValidation.py

Lines 336 to 345 in e336bad

	self.addQuery(
	lambda: (
	self.cs[globalSettings.CONF_ZONING_STRATEGY] == "manual"
	and not self.cs["zoneDefinitions"]
	),
	"`manual` zoningStrategy requires that `zoneDefinitions` setting be defined. Run will have "
	"no zones.",
	"",
	self.NO_ACTION,
	)

And they are really simple ["zone1:loc1,loc2,loc3", "zone2:loc5,loc6"]:

armi/armi/reactor/zones.py

Lines 482 to 491 in e336bad

	def _buildManualZones(core, cs):
	runLog.extra(
	"Building Zones by manual zone definitions in `zoneDefinitions` setting"
	)
	stripper = lambda s: s.strip()
	zones = Zones(core, cs)
	# read input zones, which are special strings like this: "zoneName: loc1,loc2,loc3,loc4,..."
	for zoneString in cs["zoneDefinitions"]:
	zoneName, zoneLocs = zoneString.split(":")
	zoneLocs = zoneLocs.split(",")

I think we can re-use nearly all of that. The only thing we need to add in is some way to say if the Zone is a group of Assembly or Block objects.

We will need to change the default to None or maybe "manual" or something.