-
Notifications
You must be signed in to change notification settings - Fork 90
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 components to have children in support of particle fuel modeling #702
base: main
Are you sure you want to change the base?
Changes from all commits
8244e23
91fc065
6f66097
cc1d8db
b5ef213
c91bbab
12d316b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -609,30 +609,42 @@ def getNuclides(self): | |
|
||
This includes anything that has been specified in here, including trace nuclides. | ||
""" | ||
return list(self.p.numberDensities.keys()) | ||
nucs = set(self.p.numberDensities.keys()) | ||
if self._children: | ||
childNuclides = set(composites.Composite.getNuclides(self)) | ||
nucs |= childNuclides | ||
return nucs | ||
Comment on lines
-612
to
+616
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern on getting nuclides just in the component vs. component and all the children There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah that's a good concern. I'd hate to make you basically re-write these methods for accessing the details of the matrix itself. We could add in a flag or something as a method argument? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
def getNumberDensity(self, nucName): | ||
def getNuclideNumberDensities(self, nucNames): | ||
""" | ||
Get the number density of nucName, return zero if it does not exist here. | ||
Return a list of number densities for the nuc names requested. | ||
|
||
Parameters | ||
---------- | ||
nucName : str | ||
Nuclide name | ||
If this Component has children, then homogenize their number densities in. | ||
|
||
Returns | ||
------- | ||
number density : float | ||
number density in atoms/bn-cm. | ||
Component ndens is unique in that it combines its own actual composition | ||
with that of its potential children | ||
""" | ||
return self.p.numberDensities.get(nucName, 0.0) | ||
|
||
def getNuclideNumberDensities(self, nucNames): | ||
"""Return a list of number densities for the nuc names requested.""" | ||
return [self.p.numberDensities.get(nucName, 0.0) for nucName in nucNames] | ||
|
||
def _getNdensHelper(self): | ||
return dict(self.p.numberDensities) | ||
if self._children: | ||
childDens = dict( | ||
zip( | ||
nucNames, | ||
composites.Composite.getNuclideNumberDensities(self, nucNames), | ||
) | ||
) | ||
# get volume of children by using the composite method which loops over all children | ||
childVol = composites.Composite.getVolume(self) | ||
# get self volume by calling the shape-specific volume method of the background shape | ||
totalVol = self.getVolume() | ||
childFrac = childVol / totalVol | ||
selfFrac = 1.0 - childFrac | ||
return [ | ||
self.p.numberDensities.get(nucName, 0.0) * selfFrac | ||
+ childDens.get(nucName, 0.0) * childFrac | ||
for nucName in nucNames | ||
] | ||
else: | ||
return [self.p.numberDensities.get(nucName, 0.0) for nucName in nucNames] | ||
Comment on lines
+628
to
+647
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would a plugin be able to get the number densities of just the matrix component with this change? On one hand this is consistent with how the design of the composite tree: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great point. You would have to access the matrix's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ntouran is
still the recommended approach here? I've been testing this out over the last few days and had a hard time remembering this thread. Would something like |
||
|
||
def setName(self, name): | ||
"""Components use name for type and name.""" | ||
|
@@ -1019,8 +1031,20 @@ def mergeNuclidesInto(self, compToMergeWith): | |
) | ||
|
||
def iterComponents(self, typeSpec=None, exact=False): | ||
if self.hasFlags(typeSpec, exact): | ||
yield self | ||
""" | ||
Yield the component or its direct child components. | ||
|
||
Notes | ||
----- | ||
This could probably be made recursive to allow arbitrary depth, but it's more complex | ||
""" | ||
if self._children: | ||
for child in self._children: | ||
if self.hasFlags(typeSpec, exact): | ||
yield child | ||
else: | ||
if self.hasFlags(typeSpec, exact): | ||
yield self | ||
Comment on lines
+1045
to
+1047
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This removes the ability for if self.hasFlags(typeSpec, exact):
yield self
# If no children, this loop isn't executed
for child in self._children:
for c in child.iterComponents(typeSpec, exact):
yield c might pick up the arbitrary depth and also return the child component based on the child's flags not the flags of the parent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. very nice, that does seem better. I'll try your suggestion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok so I tried With that suggested iterComponents change, and my I tried this kind of thing for a while and just couldn't quite get it working. I'm sure it can be done... it's just hard.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ooooo that's rough. Looks like the problem could stem from
Comment on lines
+1041
to
+1047
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ntouran we've found that the mass of the matrix material is not accounted for when doing We modified the triso mass test blueprint file such that each element only appears in one # A full blueprint file that contains assems made with ComponentGroups
blocks:
fuel2: &block_fuel2
triso compact:
shape: Circle
blends:
- triso particle
blendFracs:
- 0.70
material: Graphite
Tinput: 600.0
Thot: 600.0
mult: 1
id: 0.0
od: 1.2446
duct:
shape: Hexagon
material: Cu
Tinput: 600.0
Thot: 600.0
ip: 9.0
mult: 1.0
op: 10.0
coolant:
shape: DerivedShape
material: NaCl
Tinput: 25.0
Thot: 600.0
components:
# triso dims from Table 4 in TRISO benchmark
kernel:
shape: Sphere
material: Lead
Tinput: 600.0
Thot: 600.0
id: 0.0
mult: 1.0
od: 0.010625
buffer:
shape: Sphere
material: Lead
Tinput: 600.0
Thot: 600.0
id: kernel.od
mult: 1.0
od: 0.015625
ipyc:
shape: Sphere
material: Lead
Tinput: 600.0
Thot: 600.0
id: buffer.od
mult: 1.0
od: 0.017375
sic:
shape: Sphere
material: Lead
Tinput: 600.0
Thot: 600.0
id: ipyc.od
mult: 1.0
od: 0.019125
opyc:
shape: Sphere
material: Lead
Tinput: 600.0
Thot: 600.0
id: sic.od
mult: 1.0
od: 0.021125
component groups:
triso particle:
kernel:
mult: 1
buffer:
mult: 1
ipyc:
mult: 1
sic:
mult: 1
opyc:
mult: 1
assemblies:
fuel c: &assembly_c
specifier: OC
blocks: [*block_fuel2]
height: [1.0]
axial mesh points: [1]
xs types: [A]
systems:
core:
grid name: core
origin:
x: 0.0
y: 0.0
z: 0.0
grids:
core:
geom: hex
symmetry: full
lattice map: |
OC
nuclide flags:
B: &nuc_flags {burn: false, xs: true}
C: *nuc_flags
PB: *nuc_flags
CU63: *nuc_flags
CU65: *nuc_flags
CL35: *nuc_flags
CL37: *nuc_flags
CL37: *nuc_flags
NA23: *nuc_flags There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @drewj-usnctech I will bring in your modified blueprints file and update the unit test so it fails as you've found and then try to fix. |
||
|
||
def backUp(self): | ||
""" | ||
|
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 enables individual layers to have linked dimensions, right? Nice!
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.
yeah it should. though we should make more unit tests proving it.