-
Notifications
You must be signed in to change notification settings - Fork 5
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
Different implementation methods of enrich() #38
base: main
Are you sure you want to change the base?
Conversation
Provides 5 different examples of methods of how enrich could be used by the user based on different philosophies.
|
||
|
||
# I also need to have another version of this function for the pin breeder zone. | ||
# I will if need to either make a version of this for each material I'm testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will Need to either make a version of this function for each material I'm testing. Or create a universal function that will require me to pass all the arguments already in hcpb_pin_bw() along with all 8 different materials being evaluated for HCPB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are my thoughts....
- Thank for sharing it this way as it definitely clarified your ideas/concerns.
- Sorry I didn't look at
example.py
first - my tendency is to always look at the methods that are being shown in the example before looking at the example. - There may be a hybrid approach where we allow material mixing to use multiple base material libraries, allowing us to keep things both separate and convenient.
simulation_lib = MaterialLibrary() | ||
|
||
|
||
################################### Method 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely don't like this one, and we've moved away from this basic pattern elsewhere.
enriched_mat = mdbt.Material({30060000: 0.6, 30070000: 0.4}) | ||
collapsed = mat_lib_test["Li2TiO3nat"].collapse_elements({3}) | ||
atom_frac_enriched = mdbt.enrich( | ||
collapsed.to_atom_frac(), 30000000, enriched_mat.to_atom_frac() | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see how the need to collapse from the fully processed library is cumbersome - I agree that this not ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also see that converting between mass enrichment and atom fractions is cumbersome, but could be done in a single line:
enriched_atom_fracs = mbdt.Material({'Li6' : 0.6, 'Li7' : 0.4}).to_atom_frac()
But there is the danger that the user doesn't recognize the risk of missing atom and mass fractions
enriched_mat = mdbt.Material({30060000: 0.6, 30070000: 0.4}) | ||
atom_frac_enriched = mdbt.enrich( | ||
pure["Li2TiO3nat"]["atom_frac"], 30000000, enriched_mat.to_atom_frac() | ||
) | ||
# I would need to call enrich() each time I wanted to enrich a different element | ||
mat_input3 = { | ||
"atom_frac": atom_frac_enriched, | ||
"density": 3.42, | ||
"citation": "HernandezFusEngDes_2018", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely better without the need to collapse elements, and could be streamlined with a single line to get the enriched atom fractions.
enriched_mat = mdbt.Material({30060000: 0.6, 30070000: 0.4}) | |
atom_frac_enriched = mdbt.enrich( | |
pure["Li2TiO3nat"]["atom_frac"], 30000000, enriched_mat.to_atom_frac() | |
) | |
# I would need to call enrich() each time I wanted to enrich a different element | |
mat_input3 = { | |
"atom_frac": atom_frac_enriched, | |
"density": 3.42, | |
"citation": "HernandezFusEngDes_2018", | |
} | |
enriched_atom_frac = mdbt.Material({30060000: 0.6, 30070000: 0.4}).to_atom_frac() | |
# I would need to call enrich() each time I wanted to enrich a different element | |
mat_input3 = { | |
"atom_frac": mdbt.enrich(pure["Li2TiO3nat"]["atom_frac"], "Li" , enriched_atom_frac) | |
"density": 3.42, | |
"citation": "HernandezFusEngDes_2018", | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might suggest that it makes sense to have an enrich()
method that takes a mat_data
entry instead of a material composition???
pure["Li2TiO3nat"]["mass_enrichment"] = {30000000: {30060000: 0.6, 30070000: 0.4}} | ||
# I can enrich multiple isotopes at the same time with this method | ||
material4 = mdbt.make_mat_from_atom(**pure["Li2TiO3nat"]) | ||
mat_lib_test["material4_Li2TiO3"] = material4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely see the elegance of this now, in comparison to the others.
In this case, I'd probably still encourage a pattern of making a copy of the pure
entry first:
pure["Li2TiO3nat"]["mass_enrichment"] = {30000000: {30060000: 0.6, 30070000: 0.4}} | |
# I can enrich multiple isotopes at the same time with this method | |
material4 = mdbt.make_mat_from_atom(**pure["Li2TiO3nat"]) | |
mat_lib_test["material4_Li2TiO3"] = material4 | |
enrich_mat_data["material4_Li2TiO3"] = pure["Li2TiO3nat"] | |
enrich_mat_data["material4_Li2TiO3"]["mass_enrichment"] = {30000000: {30060000: 0.6, 30070000: 0.4}} | |
# I can enrich multiple isotopes at the same time with this method | |
mat_lib_test["material4_Li2TiO3"] = mdbt.make_mat_from_atom(**enrich_mat_data["material4_Li2TiO3"]) |
density: float, | ||
citation: str, | ||
molecular_mass: Optional[float] = None, | ||
mass_enrichment: Optional[Dict[int, Dict[int, float]]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you set the default as {}
instead of None
you can avoid the if statment below and just iterate over a potentially empty dictionary.
enriched_mat = Material(enrichment_vector) | ||
atom_frac = enrich(atom_frac, element, enriched_mat.to_atom_frac()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enriched_mat = Material(enrichment_vector) | |
atom_frac = enrich(atom_frac, element, enriched_mat.to_atom_frac()) | |
enriched_atom_frac = Material(enrichment_vector).to_atom_frac() | |
atom_frac = enrich(atom_frac, element, enriched_atom_frac) |
atom_frac: Dict[Union[int, str], float], | ||
density: float, | ||
citation: str, | ||
mass_enrichment: Optional[Dict[int, Dict[int, float]]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there also be a version that takes an atom_enrichment? or is that too rare and we just make users work around it?
|
||
|
||
# Mix Materials by Volume | ||
def mix_by_volume(material_library, vol_fracs, citation, mass_enrichment=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think my problem with this is that it disrupts the cognitive flow of a simple mixing method.
What if you had a method that did the work of making the new enriched materials, and called that from the beginnng of this method, adding those materials to the library, and then the rest of the method would largely be the same as it originally was?
# pseudocode and not exactly right....
def mix_by_volume(material_library, vol_fracs, citation, mass_enrichment={}):
# this probably needs some naming convention for how the materials will be
# automatically named for use later
# OR they get added to a second local/temporary library and we figure out
# which library to use for which materials we are mixing
add_enriched_materials_to_library(material_library, mass_enrichment)
# the rest of mix_by_volume() as it already exists, but allowing multiple libraries
# e.g. mix_by_volume(base_lib, enrich_lib, vol_fracs, citation)
# or
# mix_by_volume(lib_list, vol_fracs, citation)
"mass_enrichment": { | ||
"Li2TiO3nat": {30000000: {30060000: 0.6, 30070000: 0.4}}, | ||
"Li4SiO4nat": {30000000: {30060000: 0.6, 30070000: 0.4}}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to my comment below... what if we had a method that just took a material library and this dictionary and made a temporary library with enriched materials, and then we let mix_by_volume
take multiple libraries...?
Provides 5 different examples of methods of how enrich() could be used by the user based on different philosophies. These methods are all contained within the file named example.py. The current structure of the folder is not representative of the actual implementation. The file material_db_tools.py can be used to better understand the different methods described in example.py. This PR is only to help clarify, not to be implemented as is. material_example.xml is the output form example. Which is to provide supporting evidence that each method provides the same output.