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

Monte Carlo time stepping action #6198

Merged

Conversation

ffoucart
Copy link
Contributor

@ffoucart ffoucart commented Aug 5, 2024

Proposed changes

Add an action to take a single MC time step, and a test of using this action within the mock run environment.
Only the last commit is actually relevant; all others are from PR #6138 , which is a prerequisite to this PR.

Upgrade instructions

None

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

@ffoucart ffoucart added the dependent Needs a different PR to be merged in first label Aug 5, 2024
Copy link
Member

@nilsdeppe nilsdeppe left a comment

Choose a reason for hiding this comment

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

LGTM, a few suggestions. You can squash them directly

#include "Time/Time.hpp"
#include "Time/TimeStepId.hpp"

#include "Parallel/Printf/Printf.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

remove? Leftover from debugging?


namespace Particles::MonteCarlo {

// Mutator advancing neutrinos by a single step
Copy link
Member

Choose a reason for hiding this comment

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

triple slash for dox comment

Comment on lines 74 to 82
gsl::not_null<std::vector<Packet>*> packets,
gsl::not_null<std::mt19937*> random_number_generator,
gsl::not_null<std::array<DataVector, NeutrinoSpecies>*>
single_packet_energy,
Copy link
Member

Choose a reason for hiding this comment

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

const gsl::not_null

explicit NeutrinoInteractionTable(CkMigrateMessage* msg) : PUP::able(msg) {}

using PUP::able::register_constructor;
void pup(PUP::er& p);
Copy link
Member

Choose a reason for hiding this comment

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

void pup(PUP::er& p) override;

src/Evolution/Particles/MonteCarlo/Tags.hpp Show resolved Hide resolved
@ffoucart ffoucart force-pushed the MonteCarloTimeSteppingAction branch 3 times, most recently from 3ccd592 to daebafe Compare August 28, 2024 12:52
@ffoucart ffoucart removed the dependent Needs a different PR to be merged in first label Aug 28, 2024
@ffoucart ffoucart marked this pull request as ready for review August 28, 2024 12:52
@ffoucart ffoucart force-pushed the MonteCarloTimeSteppingAction branch from daebafe to f98ab56 Compare August 28, 2024 12:56
@ffoucart
Copy link
Contributor Author

Ok, I had missed a few initially, but I think now all comments have been addressed...

@ffoucart ffoucart force-pushed the MonteCarloTimeSteppingAction branch from f98ab56 to 7c4eeb5 Compare August 28, 2024 17:33
@nilsdeppe
Copy link
Member

CI is timeout in Cylindrical BBH domain, which is unrelated.

@nilsdeppe nilsdeppe merged commit e0232b1 into sxs-collaboration:develop Aug 28, 2024
21 of 23 checks passed
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