Random Ray main function and autosetup refactoring #3733
Conversation
jtramm
left a comment
jtramm
left a comment
There was a problem hiding this comment.
Looks good -- thanks for cleaning these areas up! Just a minor comment on naming, but then should be good to go.
| void RandomRaySimulation::random_ray_adjoint() | ||
| { | ||
| if (!adjoint_needed_) { | ||
| return; | ||
| } | ||
|
|
||
| // Configure the domain for adjoint simulation | ||
| FlatSourceDomain::adjoint_ = true; | ||
|
|
||
| // Reset k-eff | ||
| domain_->k_eff_ = 1.0; | ||
|
|
||
| // Initialize adjoint fixed sources, if present | ||
| prepare_fixed_sources_adjoint(); | ||
|
|
||
| // Transpose scattering matrix | ||
| domain_->transpose_scattering_matrix(); | ||
|
|
||
| // Swap nu_sigma_f and chi | ||
| domain_->nu_sigma_f_.swap(domain_->chi_); | ||
|
|
||
| // Run a single simulation | ||
| run_single_simulation(); | ||
| } |
There was a problem hiding this comment.
It would be nice to be able to deduce from the top level calls that the adjoint simulation still fundamentally the same as the forward one, just with a few preparations made. I think the naming scheme of RandomRaySimulation::run_single_simulation() and void RandomRaySimulation::random_ray_adjoint() is a little opaque, as without digging into those functions it suggests there is a very different control flow happening for forward vs. adjoint. I might propose changing RandomRaySimulation::random_ray_adjoint() to something like RandomRaySimulation::prepare_adjoint_simulation, and then pull out the run_single_simulation();call that is inside of it into the higher level caller space, such that the control flow is more clear. I also wonder if run_single_simulation() might be changed to run_simulation()? Or, even better yet, perhaps it would be better to fold this logic into the RandomRay.simulate() function, so as to avoid having both simulate() and run_simulation() functions, which is a little confusing when looking through the API why we have both of these.
|
Thanks for the feedback @jtramm. I've implemented your suggestions, let me know what you think. |
2 participants
Description
This PR refactors the
openmc_run_random_ray()function as well as the automatic setup machinery to adhere to D.R.Y.Checklist