Repository navigation
Geometron - #2744
Geometron#2744
Conversation
|
Hm. I thought I had removed the |
|
This will be really useful -- planning to leverage this for use with random ray. Thanks for breaking it out into its own PR. Once |
|
Hey John, makes sense! In the plotting PR I had removed that exception-based approach, but somehow it made its way back in while cherry picking. Silly me. Sorry to go off the grid on this. Will hopefully be able to tie in these changes pretty soon. I've kind of wanted to try making the OpenMC geometry APIs even more generic after this, maybe removing reliance on global state a bit more so that the very robust CSG capability here can be used as a library for, say, other particle or ray-based apps. For example it would be very interesting to write some particle hydrodynamics code that uses OpenMC geometry methods. |
|
@jtramm this should be all good to review now! |
jtramm
left a comment
jtramm
left a comment
There was a problem hiding this comment.
I flagged a few questions I had for you on the mark_as_lost() strategy, but besides that, this looks great. Thanks for putting it together!
| void Geometron::mark_as_lost(const char* message) | ||
| { | ||
| fatal_error(message); | ||
| } |
There was a problem hiding this comment.
At first glance, this seems problematic as the function is very different than the Particle::mark_as_lost() version, but the Geometron version is called even when tracking MC particles in some locations. Thus, it seems like it may be the case that certain types of lost particle events that previously just resulted in a warning message (not that uncommon for super large runs) would now result in a fatal error?
| const double* xyz, int32_t* index, int32_t* instance) | ||
| { | ||
| Particle p; | ||
| Geometron p; |
There was a problem hiding this comment.
Might be good to rename the variable to better reflect the fact it is no longer a full particle (geom_state instead of p?)
There was a problem hiding this comment.
Hmm yes good point!
|
Comments should be addressed now! |
|
Alright, CI is finally passing. I found my other PC was changing the results of a projection plot test slightly and I thought it might be due to some floating point noncommutativity that this PR was (somehow?) introduction, since the overall result looked the same. I tried just pasting in the old hash of the reference test results but that includes a newline, which the test references do not. So I went back and forth a few times on that. Phew! |
paulromano
left a comment
paulromano
left a comment
There was a problem hiding this comment.
Thanks again @gridley! 🚀
Description
Cherry pick out geometron changes from #2655 to its own PR.
Checklist