Further categorizing ARMI settings?

[albeanth]

Hey all - I had a thought while reviewing the new tight coupling settings ( PR #1033) that I wanted to toss out there.

Has there been a past effort/consideration to further categorize the settings? E.g. sorting/categorizing settings like:

armi app run type:
  run type:
  ncycles:
  etc:
  
  tight coupling settings:
    tight coupling: true
    global flux:
      parameter: keff
      convergence: 1e-5
    fuelPerformance:
      etc. etc.

global flux solver:
  a list of options:
  more options:
  etc:

cross sections:
  some options:
  etc:

thermal hydraulics:
  solver type:
  some options:

fuel performance:
  solver type:
  some options:

This sort of setup might be clearer to newer users. This is opposed to the current approach where everything is listed out in alphabetical order and all mixed together where either you need to know the context of/be familiar with the setting or go look it up either in the ARMI docs or grep the App source code.

Anyway, just a thought. There are likely loads of complications that prevent this sort of thing. But I figured I'd bring it up as we try and get more and more users for ARMI apps.

[keckler]

I think this would be infinitely more convenient. I also think there is nothing preventing one from defining a setting like this, since settings can be nested and we already have examples of settings like that, for instance the cycles setting.

It would just be necessary for somebody to go through and redefine the settings in such a way, perhaps using migrations to allow the old versions to still work, and then to update all places in the code where the relevant settings are queried.

That sounds like a lot of work, but I think it is possible using only the machinery we have available now.

[john-science]

I really do like organization.

But this seems like the biggest possible API break we could think up in ARMI. Or at least, has the potential to lead to Big PRs in every repo that uses ARMI. So, backwards compatibility would be my primary concern here.

For instance, basically every ARMI plugin registers settings and uses settings. So it would be a LOT of work if we change either of those workflows.

Similarly, I think our Settings could be renamed to be better. But that would require 100 PRs, and wouldn't yield much other than niceness. So my same concern lingers there.

I am in favor of organization in all things though.

[albeanth]

@john-science If we were able to ensure backwards capability, I think it would be worth exploring. Though without a doubt, it would be a huge lift and require significant planning. That said, I think the long-term benefits for ease of use at least merit consideration and some discussion. But you're 100% right, this would just ultimately result in niceness and not really advance the capabilities of ARMI.

I think the biggest offenders that I've come across are when settings that are very specific to a run type get left behind and end up in other runs (e.g., snapshot settings in standard runs or vice-versa). But since those settings aren't used, they are just ignored and left around. It can lead to settings files becoming a bit messy and a potentially convoluted, in my opinion.

[john-science]

I like this idea. Though BOY is this the mother and father of all API-breaking changes.

Just a side note, you CAN already do this:

# armi app run type:
  run type:
  ncycles:
  etc:
  
 #  tight coupling settings:
    tight coupling: true
      etc. etc.

# global flux solver:
  a list of options:
  more options:
  etc:

# cross sections:
  some options:
  etc:

# thermal hydraulics:
  solver type:
  some options:

# fuel performance:
  solver type:
  some options:

This is not what you're asking for, but you could make the ARMI settings file more readable in like 30 seconds, with no API-breaking changes.

[john-science]

@albeanth Is the API-breaking version better than this in some way?

Because I can open up PRs to do this in every repo, right now today. With very little effort.

#just-curious

[albeanth]

it's a stop gap, sure. More formal categorization might be nice though.

Could help with settings validation. E.g., someone puts a neutronics setting in the fuel performance section. There could be a settings validator saying "hey this isn't a recognized setting for this section. it needs to go somewhere else."

there may be others that are potentially useful...?

also during case suite building, I think comments get yanked and any custom ordering gets thrown out the window in favor of alphabetization.

[keckler]

Yeah, Tony is right about the alphabeticalization.

[keckler]

alphabetization, lol. I like the longer version of the word!!

[john-science]

@albeanth You are aware that your suggestion here would mean that instead of doing:"

cs["global flux"]

we would have to do:

cs["tight coupling settings"]["global flux"]

That's how tiered settings work.

It's not my favorite thing. I mean, it would involve thousands of lines of more changed code, sure. But more over, I think it would just make settings harder to use and find in code.

[drewj-tp]

This would also let us send just specific settings to interfaces that need it. So like cs["tight coupling settings"] could be a TightCouplingSettings object, and that's all the tight couplers need to know about. Rather than passing around the entire cs object

[john-science]

Yeah, that's a fun idea.

[albeanth]

Other ideas that have popped into my head from #1914 that could happen during settings validation:

1. runLog.warning gets spit out into the stdout if there are unknown settings
2. runLog.warning gets spit out into the stdout if there are settings being used that have no effect on a given simulation

[john-science]

We actually do (1)!

armi/armi/settings/settingsIO.py

Lines 245 to 261 in 356750c

	def _checkInvalidSettings(self):
	if not self.invalidSettings:
	return
	try:
	invalidNames = "\n\t".join(self.invalidSettings)
	proceed = prompt(
	"Found {} invalid settings in {}.\n\n {} \n\t".format(
	len(self.invalidSettings), self.inputPath, invalidNames
	),
	"Invalid settings will be ignored. Continue running the case?",
	"YES_NO",
	)
	except RunLogPromptUnresolvable:
	# proceed with invalid settings (they'll be ignored).
	proceed = True
	if not proceed:
	raise InvalidSettingsStopProcess(self)

Number (2) is interesting. I'll have to think about that some. My gut check is that each setting would need some more validation logic to say if it was going to be used or not? hmm

[albeanth]

I figured 1 was in place! Score.

Yeah 2 could be tricky. Off the top of my head, I don't know of an easy way to make this happen. Though I do think categorization would help 😉