-
Notifications
You must be signed in to change notification settings - Fork 2
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
Photon Transport Simulation with Two Layers of Material & Unstructured Mesh #27
base: main
Are you sure you want to change the base?
Conversation
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.
Glad to see this is all (mostly) working
Co-authored-by: Paul Wilson <[email protected]>
- Reading total strengths directly from Source_Mesh_Reader instead of writing a new list in this script
- Brought the MeshBase object inside the loop over the energy bounds
Assign material library path to new function and delete unused lines
These two files can be imported into a neutron or photon transport problem
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.
A number of suggestions here. We should meet to discuss more.
I think all of these files should be in the WC_Layers
directory, and the neutron transport file should be updated to use the same geometry and materials scripts.
TwoLayers_Geometry.py
Outdated
@author: Anupama Rajendra | ||
""" | ||
|
||
import openmc |
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.
Not clear that we need to import openmc
here, if this method is only ever used when importing into another script that also imports openmc
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 might be missing something here, but OpenMC_PhotonTransport attempts to access openmc inside TwoLayers_Geometry even when only make_spherical_shells is imported
Source_Mesh_Reader.py
Outdated
tstt = file['tstt'] | ||
elements = tstt['elements'] | ||
tet_four = elements['Tet4'] | ||
tags = tet_four['tags'] | ||
sd = tags['source_density'][:] | ||
|
||
# Write source density to a separate file for each mesh | ||
with open(f'source_density_{file_index + 1}.txt', 'w') as source: | ||
for tet_element in sd: | ||
source.write(' '.join(map(str, tet_element)) + '\n') | ||
|
||
# Calculate summed (and individual mesh) strengths for each photon group | ||
summed_strengths = [] | ||
strengths_list = [] | ||
for group in range(photon_groups): | ||
total_strengths = np.sum(sd[:, group]) # Sum over all mesh elements | ||
summed_strengths.append(total_strengths) | ||
strengths = sd[:,group] | ||
strengths_list.append(strengths) |
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 recommend putting this in a method that does everything you need for a single file. That kind of modularity enhances readability. You might also want to separate the functions of reading from a file, processing the data, and writing to files, so methods like extract_source_data
, arrange_data
, write_source_density
Co-authored-by: Paul Wilson <[email protected]>
- Created dictionary with material densities - Created OpenMC Material and Materials objects with separate function
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.
More suggestions as you chip away at this
TwoLayers_Geometry.py
Outdated
|
||
# Create geometry | ||
#Spherical shell: | ||
def make_spherical_shell(inner_radius_W, outer_radius_W, inner_radius_C, M_1, M_2): |
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 makes multiple shells, so maybe change the name.
Could also generalize this to take an inner radius plus layers:
def make_spherical_shell(inner_radius_W, outer_radius_W, inner_radius_C, M_1, M_2): | |
def make_spherical_shells(inner_radius_W, layers): | |
''' | |
Builds a set of nested spherical shells, with an inner_radius and | |
multiple layers each with a material definition and a thickness | |
inputs | |
===== | |
inner_radius : the radius of the inner surface of the first shell | |
layers : list of tuples where each tuple is (material, thickness) | |
''' |
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 materials will need to be an input as this function is also assigning materials to the geometry. I'm wondering if this can be done with one set of radii/thickness inputs and the materials from TwoLayers_Materials.
def make_spherical_shell(inner_radius_W, outer_radius_W, inner_radius_C, M_1, M_2): | |
def make_spherical_shell(radii, materials): | |
''' | |
Takes a list of materials and radii (outermost to innermost) | |
and creates an OpenMC Geometry object. | |
element_list = list of elements (str) in order from radially outermost to innermost | |
radii = list of the radii (outermost to innermost) that serve as the boundaries of the | |
spherical shells; must be in the same order as the specified materials | |
''' | |
cells = [] | |
for index, material in enumerate(materials): | |
outer_sphere = openmc.Sphere(r = radii[index]) # add boundary_type here | |
inner_sphere = openmc.Sphere(r = radii[index + 1]) | |
element_region = +inner_sphere & -outer_sphere | |
element_shell = openmc.Cell(fill = materials[index], region = element_region) | |
cells.append(element_shell) | |
if material == materials[-1]: | |
void = openmc.Cell(fill = None, region = -inner_sphere) | |
cells.append(void) | |
geometry = openmc.Geometry(cells) | |
return geometry | |
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 agree that the materials are necessary. My version had them bound in the layers
. This kind of data structure can lead to clarity because the data gets bound together in instructive ways. An example of calling it might be:
inner_radius = 10.0
# material_a and material_b are OpenMC Materials
layers = [(material_a, 5.0), (material_b, 4.0)]
geometry = make_spherical_shells(inner_radius, layers)
The function may be:
def make_spherical_shells(inner_radius, layers):
inner_sphere = openmc.Sphere(inner_radius)
cells = [openmc.Cell(region=-inner_sphere)]
for (material, thickness) in layers:
outer_radius = inner_radius + thickness
outer_sphere = openmc.Sphere(outer_radius)
cells.append(openmc.Cell(fill = material, region = inner_sphere & -outer_sphere))
inner_radius = outer_radius
inner_sphere = outer_sphere
cells.append(openmc.Cell(region = outer_sphere)
geometry = openmc.Geometry(cells)
return geometry
(There may be minor syntax errors, but this is the basic idea)
Make dictionary of densities with ALARA element library Co-authored-by: Paul Wilson <[email protected]>
Redefine inner and outer W spheres Co-authored-by: Paul Wilson <[email protected]>
- Redefined make_spherical_shells to take layers and inner_radius as input - Deleted reference to TwoLayers_Materials to maintain generality
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.
A few more high level comments here. They will probably result in a lot of code changes, but shouldn't take too long. Conceptually, it's a lot of same changes across many files.
…Mesh_Reader_Inputs.yaml
…Input/Conversion_h5m.py
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 didn't note all the places that comments from other PR's/projects might be applied here...
WC_Layers/TwoLayers_Geometry.py
Outdated
thicknesses: thickness of each OpenMC Material | ||
inner_radius: the radius of the innermost spherical shell | ||
''' | ||
layers = zip(materials, thicknesses) |
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.
Might be better to zip these together before calling the function (or as you call the function). This reinforces that these need to be bound to each other in this way.
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 function now takes layers as input.
WC_Layers/Source_Mesh_Reader.py
Outdated
inputs = yaml.safe_load(yaml_file) | ||
|
||
source_mesh_list = inputs['source_meshes'] | ||
num_elements = inputs['num_elements'] |
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.
Is the number of mesh elements defined by the mesh? Doesn't this introduce the possibility that there is a discrepancy introduced by the user?
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.
We set it up this way so that sd_list
could be a numpy array instead of a list. Specifying the wrong value of either the number of mesh elements or photon groups will produce an error
sd_list = np.ndarray((len(source_mesh_list), num_elements, photon_groups)) | ||
for source_index, source_name in enumerate(source_mesh_list): | ||
file = h5py.File(source_name, 'r') | ||
sd_list[source_index,:] = file['tstt']['elements']['Tet4']['tags']['source_density'][:] |
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.
sd_list
is initialized as three-dimensional, but you only refer to 2 dimensions here. That may be OK if both colons refer to 2-D data?
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.
Yes, both colons refer to the 2D photon_groups X num_elements data. This is just making a list of the 2D data for each decay step
WC_Layers/Source_Mesh_Reader.py
Outdated
|
||
source_mesh_list = inputs['source_meshes'] | ||
num_elements = inputs['num_elements'] | ||
photon_groups = inputs['photon_groups'] |
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.
Isn't the number of groups already defined internally to the mesh file?
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.
Same as mentioned above regarding the number of mesh elements
WC_Layers/Photon_TallytoVtk.py
Outdated
summed_axes_dose_filter = inputs['summed_axes_dose_filter'] | ||
cd = calculate_dose(rs[2], summed_axes_dose_filter) | ||
|
||
summed_axes_mesh_filter = inputs['summed_axes_mesh_filter'] |
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.
Looking at the YAML, the meaning/interpretation of this is not very obvious/clear.
WC_Layers/OpenMC_PhotonTransport.py
Outdated
total_filter = openmc.MeshFilter(total_mesh) | ||
|
||
photon_tally = openmc.Tally(tally_id=1, name="Photon tally") | ||
photon_tally.scores = ['flux', 'elastic', 'absorption'] |
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.
Are these scores relevant/interesting for your photon tally?
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.
Probably want the bounds
from your YAML file
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'm getting rid of the elastic tally as it is 0, but it might be interesting to see how the flux and absorption spectra evolve relative to each other (once bounds
is applied as an energy filter)
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.
photon_tally
is now set up to record flux and absorption over bounds
, but I'm wondering if the absorption score can just be moved to spectrum_tally
instead
WC_Layers/OpenMC_PhotonTransport.py
Outdated
OpenMC Tallies object | ||
''' | ||
particle_filter = openmc.ParticleFilter('photon') | ||
total_filter = openmc.MeshFilter(total_mesh) |
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'm not sure what total
means in these variable names?
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 meant 'total' as in the unstructured mesh encompasses the full geometry, but I realize that's not clear from the variable name. Renamed total_mesh
as unstructured_mesh
.
dose_filter = openmc.EnergyFunctionFilter(dose_energy, dose) | ||
|
||
# An energy filter is created to assign to the flux tally. | ||
energy_filter_flux = openmc.EnergyFilter.from_group_structure("VITAMIN-J-42") |
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'm not sure this is relevant for photons??
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 thought the 42-group structure was for photons and the 175 one for neutrons...or should this change to match the group structure from ALARA?
WC_Layers/OpenMC_PhotonTransport.py
Outdated
|
||
spectrum_tally = openmc.Tally(tally_id=2, name="Flux spectrum") | ||
# Implementing energy and cell filters for flux spectrum tally | ||
spectrum_tally.filters = [cell_filter, total_filter, energy_filter_flux, particle_filter, dose_filter] |
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.
maybe you should apply the dose_filter
to the photon_tally
instead of the spectrum_tally
?
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 photon_tally is changed to include an energy filter with the ALARA bounds, I'm not sure it would matter which tally the dose filter is associated with
Co-authored-by: Paul Wilson <[email protected]>
This model currently uses the relative source intensities between different photon groups as the source strengths (and samples equally between individual mesh elements for any given energy group). However, it should be possible to add spatial variation of source intensity onto the MeshSpatial distribution.