-
Notifications
You must be signed in to change notification settings - Fork 535
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
New Feature Development: The Implementation of chord length sampling (CLS) method for stochastic media( #3286 ) #3305
base: develop
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.
IMO, this should not be its own type of cell, but rather a functionality introduced to the cell base class. I think so far, the role of a "cell" has been to delineate a region of space, and store some extra values.
There is no reason why a DAGCell should not be able to do chord length sampling, but that functionality has been artificially restricted here. The sub-cell material model might be better off as an empty pointer added to the cell class. There are multiple ways to have a sub-cell heterogeneity model. Exponential distribution chord length sampling is one of them, and I believe there are other stochastic media models out there which would use a similar interface.
I'd love to hear someone else's opinion on how this should be architected. Maybe we should clarify what the abstract role of a Cell
is a little better.
include/openmc/cell.h
Outdated
// Set the translation vector for the filled universe | ||
void set_translation(const vector<double>& trans) { translation_ = trans; } | ||
// Using base class methods without modifying | ||
using CSGCell::bounding_box; |
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.
These are all unnecessary with public inheritance.
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.
Thank you very much for your guidance. According to your suggestion, I will rethink how to construct this method.
My current plan is to first implement the CLS method in CSGCell and then expand it to DAGCell after verification.
My preliminary idea is as follows:
- Add a new abstract class called
subcellModel
, and it will be added to the cell class as an empty pointer. - Create a new class called
CLSModel
, which inherits from thesubcellModel
class. All calculations related to CLS chord length sampling will be implemented in this class. - Modify the transport logic to implement the CLS method.
If you have any other suggestions, please inform me in a timely manner. I will do my utmost to complete them.
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.
By the way, do I need to close this PR and start another one, or should I continue to make modifications in this PR?
Thanks for this contribution @cn-skywalker! It's exciting that we can make this method available. It's come across my desk a few times and I like it. I'll second @gridley's comments on generalizing the method to support both CSG and DAGMC/CAD cell types. The additional pointer specifying that the cell is filled with a CLS distribution could be a good approach. I'm thinking of this as an alternative cell fill type in the hope that we can implement it in this way without affecting higher level geometry methods (i.e.
Please feel free to make updates in the same PR, yes! What I'll do for now is make this a draft PR that you can mark as ready for review when the time comes. |
Currently, I have developed the Stochastic_Media class modeled after the Material class, and plan to implement it within the material input card section.I performed a local reset to the develop branch to revert previous inappropriate modifications, resulting in a non-linear commit history. The latest three commits currently reside on the develop branch as functional updates. |
and improve stochastic media reading logic
DescriptionI added test input cards for CLS_media which demonstrate that the relevant parameters can be correctly read. The next step will be to implement stochastic media as a class similar to materials and universes, which can be filled into cells. Test Case Example <CLS_media id="400" particle_material="100" matrix_material="200" radius="0.1" pack_fraction="0.3"/>
<CLS_media id="500" particle_material="100" matrix_material="200" radius="0.1" pack_fraction="0.3"/> Next PlanEnable stochastic media to be assigned to cells like materials and universes |
…odule to separate stochastic_media module
…al sampling and distance calculation
…seed for clss Stochastic_Media to sample the material
Development of Stochastic Media Functionality(1) Integration as Cell Filling Type
(2) Modifications to
|
3. Verification CasesI have added the following test cases for validation: (1) Comparison with Homogenization Method (VHM)
This result looks great, but the following validation confuses me (2) Comparison with Explicit Particulate Model (RSA)
I've put the corresponding input cards in this zip file ❓ Unexpected BehaviorThe significant discrepancy between CLS and RSA results raises several concerns:
🆘 Community Assistance Request@pshriwise @gridley This question has puzzled me for a long time. Any community discussions or assistance would be greatly appreciated! 🙏 |
Very interesting, thank you for sharing your detailed findings! Maybe running a stochastic volume calculation to check that the material volumes are truly equivalent would be one step towards a solid match against the explicit geometry model. It seems that the difference in k is larger than you'd typically expect from a fine-scale heterogeneity effect, so I would guess this relates to material volume conservation. I'd love to dive deeper into this method with you in the future, but won't be able to help much on this for now, sorry. |
Additional Debugging DetailsVolume Analysis for UO₂ Dispersed in Zirconium
Results Comparison
Visualization@pshriwise @gridley The results showed that the difference in material volume among the methods was not significant. Now only modified the material switching logic in related functions .No changes made to other core modules. Have I overlooked any critical operations during this modification?Any other suggestions to help locate the bug? |
Description
In #3286, the Chord Length Sampling (CLS) method was described for enhancing modeling capabilities in OpenMC. This PR begins the implementation of this feature.
The main changes in this PR include:
Cell
class: Addedparticle_radius_
andfill_ratio_
attributes to store the radius and fill ratio of dispersed particles.CLSCell
class: Inherits from theCSGCell
class. Added theset_translation
function to dynamically modifytranslation_
.read_cells
function: Updated to initialize theCLSCell
class.Test Case
This test case describes a scenario where Universe 1 (composed of cell1 and cell2) is randomly dispersed in cell3. A new
dispersion
tag is added to store theparticle_radius
andfill_ratio
variables.Test Results
Testing confirmed that the newly added
CLSCell
class can correctly read thedispersion
tag.Future Development Plan
To minimize modifications to the existing transport logic, I plan to implement on-the-fly modeling of dispersed particles by dynamically adjusting the
translation_
property of cell3 during runtime.Acknowledgments
As this is my first attempt at developing such a significant feature, any discussions and guidance would be greatly appreciated!
Checklist