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

Rewriting SS Model as a Set of Functions #18

Open
wants to merge 72 commits into
base: main
Choose a base branch
from

Conversation

anu1217
Copy link
Contributor

@anu1217 anu1217 commented Aug 26, 2024

Adding OpenMC scripts with functions that create .xml files

Adding OpenMC scripts with functions that create .xml files
…MC_xml.py

Adding additional folder to store scripts
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments here to get started

OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
OpenMCSphericalShellFunctions/OpenMC_xml.py Outdated Show resolved Hide resolved
anu1217 and others added 2 commits August 27, 2024 11:58
@anu1217 anu1217 marked this pull request as draft November 27, 2024 01:48
Replacing with individual files that make OpenMC Materials, Geometry, and Model
…Functions/OpenMC_Model/OpenMC_SS_Geometry.py
…Functions/OpenMC_Model/OpenMC_SS_Material.py
…lit_into_Functions/OpenMC_Model/OpenMC_Source_Tallies_Model.py
@anu1217
Copy link
Contributor Author

anu1217 commented Nov 29, 2024

I've broken up the existing Spherical Shell script into a Materials, Geometry, and a Source/Tallies/Model. The depletion and post-processing will come in subsequent PRs

@anu1217 anu1217 marked this pull request as ready for review November 29, 2024 05:49
@anu1217 anu1217 changed the title Adding OpenMC_xml.py Rewriting SS Model into a Set of Functions Nov 29, 2024
@anu1217 anu1217 changed the title Rewriting SS Model into a Set of Functions Rewriting SS Model as a Set of Functions Nov 29, 2024
@gonuke
Copy link
Member

gonuke commented Dec 2, 2024

I think I anticipated that this would replace the version in SphericalShell/OpenMC_Input, rather than keeping two versions of the same thing. This new code implements best practices so should be a reasonable replacement of the old code.

@anu1217 anu1217 marked this pull request as draft December 5, 2024 21:01
@anu1217
Copy link
Contributor Author

anu1217 commented Dec 5, 2024

I'll add the depletion and post-processing scripts here as well since SS_Activation_OpenMC is getting replaced

…enMC_SS_YAML.yaml to SphericalShell/Split_into_Functions/OpenMC_SS_YAML.yaml

Move YAML to directory above model & depletion
This is being replaced by a set of xml files that make the model, perform depletion, and analyze results
…n/SS_Depletion.py to SphericalShell/OpenMC_Input/Depletion
…enMC_SS_Geometry.py to SphericalShell/OpenMC_Input/Model
…hell/OpenMC_Input/OpenMC_Depletion/SS_Depletion
…enMC_SS_Material.py to SphericalShell/OpenMC_Input/SS_Material.py
@anu1217 anu1217 requested a review from gonuke December 23, 2024 01:09
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets better/cleaner with each step. Plugging away at more readability, streamlining and separation of concerns

'''
inputs:
element: elemental symbol of chosen element (str)
density_dict: dictionary with key = elemental symbol & value = density [g/cm^3]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your docstring doesn't include the geometry info.... but I wonder if that should be done elsewhere. I am guessing this is only necessary for the depletion/activation calculation. Should we do it in a different way to keep the separation of concerns of just making a new material?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The volume is used for depletion and during post-processing, where flux data is normalized by volume. Calculating the volume separately for both of these might be best for separation of concerns...I think I was trying to avoid having to import the thickness/radius on each occasion

SphericalShell/OpenMC_Input/SS_Model_to_xml.py Outdated Show resolved Hide resolved
SphericalShell/OpenMC_Input/SS_Model_to_xml.py Outdated Show resolved Hide resolved
SphericalShell/OpenMC_Input/SS_Model_to_xml.py Outdated Show resolved Hide resolved
nuclide_array = np.append(nuclide_array, nuclide)

# Remove None element from array initialization
nuclide_array = [nuc for nuc in nuclide_array if nuc is not None]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we end up with None in this list/array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from initialization of the associated np.ndarray


# Remove None element from array initialization
nuclide_array = [nuc for nuc in nuclide_array if nuc is not None]
return nuclide_array, materials_object, dep_results, time_steps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only returns the materials_object from the last time step, is that intentional/necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand, it doesn't matter which time step materials_object is from, as it is only used in conjunction with get_atoms(), which gives nuclide concentration vs. time based on the material the specified nuclide came from. I see the same results if I specify a material id for get_atoms instead of a materials object.

import numpy as np
import matplotlib.pyplot as plt

def extract_nuclides(dep_file_path, time_units, depletable_mat_index) :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the purpose of this is to collect a list of all the nuclides that appear, excluding the list of stable nuclides at beginning of life. Is that right? Do don't seem to actually record/return any data about those nuclides as a function of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function just stores all the new nuclides that were created (and any unstable ones at the beginning of operation), so that they can be easily retrieved later. The get_atoms method (used in plot_save_data) returns nuclide concentration as a function of time.

@anu1217 anu1217 requested a review from gonuke January 10, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants