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

Allow querying materials by elements #95

Open
MicahGale opened this issue Aug 1, 2022 · 22 comments · May be fixed by #608
Open

Allow querying materials by elements #95

MicahGale opened this issue Aug 1, 2022 · 22 comments · May be fixed by #608
Assignees
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers
Milestone

Comments

@MicahGale
Copy link
Collaborator

I think that the Materials collection should have a filtering generator similar to how Surfaces has one for surface type (e.g, surfaces.pz).

I envisage that you should be able to filter by containing elements. e.g., materials.u or materials.uranium would return all materials that contain 92***.**c, etc. I can't decide if it should be done by element atomic symbol or by element name. Name is easier to read and doesn't require memorizing atomic symbols. Symbols would be shorter and avoids the whole controversy of UK-English spelling aluminum wrong.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 21, 2023, 13:39

Suggest adding a .get_containing() method, which takes a string and returns a set of materials matching.

The method can convert "Al", "13", "aluminum", and "AlUmInIuM" to 13*** by means of a lookup table.

@MicahGale
Copy link
Collaborator Author

Great idea. Some of that framework already exists with the new Element class. I think it would also be helpful to implement this on an iterable, so you can say:

get_containing({"fe", "ni", "cr"}) to get stainless steels.

@MicahGale
Copy link
Collaborator Author

maybe add a threshold option to filter out miniscule traces. Like get_containing({"fe", "cr", "al"}, threshold = 0.05) to get FeCrAl, but not depleted fuel.

@MicahGale
Copy link
Collaborator Author

It might also be helpful to be able to get stuff by composition. Like being able to filter U-10Zr from U-20Zr. Maybe something like .get_composing({"u": material.BAL, "Zr":0.10}) for U-10Zr.

It might also be helpful to do enrichment. I think we could use a similar function but also accept isotopes.

.get_composing({"U235": 0.15})

@MicahGale
Copy link
Collaborator Author

I feel like @bascandr probably has seem interesting edge cases.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 21, 2023, 16:14

threshold will be a nice feature

I'm thinking limit it to one argument and let the user to their set logic, like:

fecral = (mats.get_containing("fe") & mats.get_containing("cr") & mats.get_containing("al")) - mats.get_containing("U")
#
heu = mats.get_containing("U235", threshold=0.205) | mats.get_containing("U233", threshold=0.205) 

@MicahGale
Copy link
Collaborator Author

How many users will be able to build set logic like this? I think it's very elegant to support, but I think supporting multiple arguments might still be more intuitive.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 21, 2023, 16:26

Supporting multiple arguments, always doing the logical AND, would be nice. I do think it should return sets; most MCNP users understand set logic by virtue of using the geometry, and sets will be ideal for materials with different thresholds and for OR and NOT operations.

@MicahGale
Copy link
Collaborator Author

TIL: Binary operators are implemented for sets. Why was this not in the tutorial.

But I think your function definition is the way forward.

This look good?

def get_containing(filter, threshold=0.0):
#excuse lack of indent
"""
Filters all materials by the given element(s) or isotope(s), and return a set of match materials.

This can accept multiple filter formats. The most basic form is a string that represents and element or isotope. Supported element filters are: the element name, element symbol, and Z number.
Isotopes can be formatted almost any way. If an iterable is passed all elements must be a string.
Each string will be parsed, and materials which contain all matching components will be returned.

Examples filters:
 * "U"
 * "u" 
 * "uranium"
 * "92"
 * "aluminium"
 * "aluminum"
 * "u235"
 * "92235"
 * "235u"
 * "u-235"
 * "Co-60m"
 * "60m2-Co"

:parameter filter: the filers to apply see above.
:type filter: str or iterable
:param threshold: the minimum threshold that a filter must pass for a given material [at. % or wt. %]. Must be in [0,1].
:type threshold: float
:returns: a set of all matching materials.
:rtype: set
"""

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 22, 2023, 12:12

That'd be sweet.

Important nuance: MCNP materials are not necessarily normalized. Should MCNPy respect the atom/weight fractions from the user's input deck, or normalize when considering the threshold?

@MicahGale
Copy link
Collaborator Author

-_-. Material definitions that are not normalized are .... just no.

But you have a good point. I keep going back and forth. I am leaning towards just normalize it.

@MicahGale
Copy link
Collaborator Author

In GitLab by @bascandr on Feb 22, 2023, 12:23

But I'm lazy and I like all of my uranium isotopes to add up to 1.00 during enrichment scoping. It makes the material cards so much easier to look at.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 22, 2023, 12:30

There are a few good reasons, such as representing atom % with the actual number densities of the constituent nuclides. Also people who represent H2O as 8016.00c 0.333333 1001.00c 0.666667 don't get invited to parties.

@MicahGale
Copy link
Collaborator Author

@bascandr, @tjlaboss I like your use cases. What I don't like are the ones normalized to the density of the end use case. Just why?

@MicahGale
Copy link
Collaborator Author

Other topic. Is still worth while to add the generators: materials.u and materials.uranium?

They seem relatively trivial to add.

@MicahGale
Copy link
Collaborator Author

Quick thought on returning sets. This would break behavior with how slices work. Slices return a new collections object with only those matching items. This isn't too much of an issue, but it does cause some inconsistency. Would it be worthwhile to standardize? Other option: we make numberedObjectCollections behave like sets. They basically are weird ordered sets already...

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Feb 22, 2023, 12:52

I do like the option of having a NumberedObjectCollection behave as a set. Since the collections can come from different models, though, the standard Python set logic might not be appropriate for identically-numbered but unequal objects. Three solutions:

  • Only allow set operations for NumberedObjectCollection instances linked to the same problem.
  • Allow them, but warn for every conflict.
  • Don't implement set operations, and just have the user manually do set(num_obj_col) if they want.

@MicahGale
Copy link
Collaborator Author

Just going to mention #61 here.

But this is very much an issue that I have been grappling for awhile, that lead to all those weird scripts that can't be upgraded that I showed you today @bascandr. This needs to ultimately get resolved by #22.

My overall view is that we should allow different problems. I think we need to be able to spot ostensibly identical objects to merge (for instance two files with common heritage). If they are the same they should merge. This then deals with problems of pointers though. Maybe we need to implement a hidden object state that just engages passthrough mode, probably knock that sort of thing out along with #83. But if there is a number collision of dissimilar objects I think the RHS (to be consistent) should be renumbered with a warning. Though this could cause issues if users didn't intend that. Maybe we need a system to rollback changes? Like a database? That would be actually really nice:

try: 
   problem.start_transaction()
   ... do error prone thing...
   problem.commit()
except as e:
   problem.roll_back()
   raise e

Realistically this would be a with statement most likely.

Suffice it to say I think we enough discussion to open a few new issues.

@MicahGale
Copy link
Collaborator Author

marked this issue as related to #97

@MicahGale
Copy link
Collaborator Author

marked this issue as related to #98

@MicahGale
Copy link
Collaborator Author

See #97, #98.

@MicahGale
Copy link
Collaborator Author

unassigned @MicahGale

@MicahGale MicahGale mentioned this issue Sep 4, 2024
35 tasks
@MicahGale MicahGale self-assigned this Sep 4, 2024
@MicahGale MicahGale modified the milestones: Summer-sprint, 1.0.0-alpha Nov 8, 2024
@MicahGale MicahGale linked a pull request Dec 5, 2024 that will close this issue
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant