celeritas-project / celeritas

Celeritas is a new Monte Carlo transport code designed to accelerate scientific discovery in high energy physics by improving detector simulation throughput and energy efficiency using GPUs.
https://celeritas-project.github.io/celeritas/user/index.html
Other
58 stars 32 forks source link

Add neutron inelastic process #1187

Closed whokion closed 3 months ago

whokion commented 3 months ago

Description

This MR includes

whokion commented 3 months ago

@sethrj, @amandalund : this MR is just to pick up most of #1150 codes (sorry for losing all review comments with this new MR) - will add remaining codes later as they are not necessary pieces for initial code base at the moment. Only new addition to the earlier MR are for nucleon-nucloen cross sections in 1) NeutronInelasticModel.cc and 2) NucleonNucleonXsCalculator.hh. Thanks for your review.

whokion commented 3 months ago

@sethrj I resolved conflicts and updated codes reflecting @amandalund's review comments, then rebased the local neutron-inelastic with origin/develop (which was successful). Now, git push still fails with

To https://github.com/whokion/celeritas
 ! [rejected]            neutron-inelastic -> neutron-inelastic (non-fast-forward)
error: failed to push some refs to 'https://github.com/whokion/celeritas'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.

I remember that you suggested to merge instead of push --force. After merging the branch to the local develop and pushed the develop to the remote develop, I tried to push the local neutron-inelastic to the remote neutron-inelastic, which still fails (i.e., error above). Probably I missed something here. Thank you for your help.

sethrj commented 3 months ago

@whokion Please don't do any rebasing once reviews have started. You should instead git merge. If you can push your current branch to a new neutron-inelastic-temp I can try to fix it. Or you could try git rebase -i --onto aa5f3b7 upstream/develop to see if that puts your changes back on what's already pushed here. (And then follow up with git merge upstream/develop.)

As a reminder for why merging is preferred once a PR has left draft mode: rebasing destroys the CI logs and messes with the review history.

whokion commented 3 months ago

Thanks @sethrj! Good to know and to learn. I just to push a new neutron-inelastic-temp as the other suggested instruction seems not working for me. Sorry and thanks again for your help.

whokion commented 3 months ago

@sethrj Thanks for reviewing and extra comments. Regarding to the consolidation of the Xs calculators, your suggestion looks good. However, I still want to postpone it to later as there may be an extension for the inelastic interaction cross sections as there are several special elements which have isotope-dependent cross sections. Once the capture cross sections (which are mostly isotope-dependent and are different structure from elastic ones), I would reconsider which way is better to consolidate; elastic vs. inelastic (element-dependent) or inelastic vs. capture (isotope-dependent). Anyway, I pushed a branch name, neutron-inelastic-temp2 (and sorry again for the extra rebase/merging treatment).

amandalund commented 3 months ago

@sethrj did you incorporate the changes from the neutron-inelastic-temp2 branch?

sethrj commented 3 months ago

God damn it, I pushed to my origin instead of Soon's. Thanks for the catch @amandalund .