Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

material modifications table in docs is missing options #1437

Closed
keckler opened this issue Oct 17, 2023 · 4 comments · Fixed by #1441
Closed

material modifications table in docs is missing options #1437

keckler opened this issue Oct 17, 2023 · 4 comments · Fixed by #1441
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers low priority

Comments

@keckler
Copy link
Member

keckler commented Oct 17, 2023

This table lists all the material modifications available for each of the material classes that have them:

image

Some of the entries in this table are missing options. For instance, all subclasses of material.FuelMaterial should have the options class1_custom_isotopics, class2_custom_isotopics, class1_wt_frac. Right now those are not listed for many of the fuel materials because we only search through the applyInputParams method at the lowest level of the MRO for each of the materials.

This can be changed by editing the code snippet here:

.. exec::
from armi.materials import Material
from tabulate import tabulate
data = []
for m in Material.__subclasses__():
numArgs = m.applyInputParams.__code__.co_argcount
if numArgs > 1:
modNames = m.applyInputParams.__code__.co_varnames[1:numArgs]
data.append((m.__name__, ', '.join(modNames)))
for subM in m.__subclasses__():
num = subM.applyInputParams.__code__.co_argcount
if num > 1:
mods = subM.applyInputParams.__code__.co_varnames[1:num]
data.append((subM.__name__, ', '.join(mods)))
data.sort(key=lambda t: t[0])
return tabulate(headers=('Material Name', 'Available Modifications'),
tabular_data=data, tablefmt='rst')

We did some work on this same table back in #431 to make all of the relevant materials show up. We should have caught this issue at that time as well, but I wasn't as familiar with the material classes back then.

@keckler keckler added documentation Improvements or additions to documentation good first issue Good for newcomers low priority labels Oct 17, 2023
@keckler
Copy link
Member Author

keckler commented Oct 17, 2023

Possibly related, I can't tell...

This variable defined in the materials package is never used anywhere in the framework nor in our internal applications. I think it is defunct?

# the co_varnames attribute contains arguments and then locals so we must restrict it to just the arguments.
AVAILABLE_MODIFICATION_NAMES = {
name
for subclass in dynamicImporter.getEntireFamilyTree(Material)
for name in subclass.applyInputParams.__code__.co_varnames[
: subclass.applyInputParams.__code__.co_argcount
]
}
AVAILABLE_MODIFICATION_NAMES.remove("self")

@john-science
Copy link
Member

This variable defined in the materials package is never used anywhere in the framework nor in our internal applications. I think it is defunct?

I added this to a cleanup PR: #1439

@keckler
Copy link
Member Author

keckler commented Oct 18, 2023

Also worthy to mention that a couple materials are in this table twice somehow?

@john-science
Copy link
Member

Also worthy to mention that a couple materials are in this table twice somehow?

UraniumOxide and ThoriumOxide each appear twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers low priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants