terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
224 stars 87 forks source link

Force Settings to have descriptions #1734

Closed john-science closed 2 months ago

john-science commented 3 months ago

I have often had to field questions about settings, because they did not have proper descriptions. This is a sign that settings without descriptions are problematic for our user base.

I propose we add an assertion to the settings constructor, to demand that a setting has a valid description, as is being done with parameters.

This is an API-breaking change for our downstream repos, so we will have to take that into account with our timeline.

The following ARMI settings do not currently have descriptions:

Update: All settings in ARMI now have descriptions.

ntouran commented 3 months ago

Seems like a good suggestion.

FWIW all those settings (except debug) are fuel management control parameters that have been used in the past during multi-objective parameter sweeps to hunt for and optimize the fuel management programs.

john-science commented 3 months ago

~@ntouran Is it possible debug is entirely unused, and has been replace by verbosity and branchVerbosity?~

I did some research and found that was the case. The setting has since been removed.

jakehader commented 2 months ago

Is this ticket completed @john-science?

john-science commented 2 months ago

Is this ticket completed @john-science?

Nope!

All the Settings in ARMI have descriptions now. But there is still a DeprecationWarning in place for settings that don't. This ticket will be completed when we move the current DeprecationWarning to a hard error.

A cursory inspection of important downstream projects says I could make that change now, but I will give it another ~2 weeks to give people warning that we are about to make this change.

Thanks!