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

setting do_cic_particles = 1 and doPoissonSolve = 0 causes obscure error message #832

Open
chongchonghe opened this issue Dec 22, 2024 · 3 comments

Comments

@chongchonghe
Copy link
Contributor

Describe the bug
When we set do_cic_particles = 1 in a test problem (e.g., binary_orbit.cpp) and accidently set doPoissonSolve_ = 0 (the default), we get this error which is hard to debug:

0::Assertion `i < (this->std::vector<T, Allocator>::size())' failed, file "/Users/cche/softwares/quokka/quokka.copy3/extern/amrex/Src/Base/AMReX_Vector.H", line 35 !!!

To Reproduce
Steps to reproduce the behavior:

  1. Compile this problem 'BinaryOrbitCIC' whiling setting sim.doPoissonSolve_ = 0;.
  2. Run this problem
  3. See error

Suggestion
This error occurs because the code tries to access phi in kickParticlesAllLevels() but phi is not initialized unless the Poisson solver is turned on. So, my suggestion is to abort the program if do_cic_particles = 1 && doPoissonSolve_ = 0. With the new particle interface, we should abort the program if we turn on any of the particle types with gravity and do not turn on Poisson solver.

Additional context
Add any other context about the problem here.

@chongchonghe chongchonghe added the bug: wrong answer/failure/crash Something isn't working label Dec 22, 2024
@dosubot dosubot bot added the particles label Dec 22, 2024
@markkrumholz
Copy link
Collaborator

Agree that that is a useful check to implement in the new particle architecture. Same thing for particle types that emit radiation and in simulations that are run with radiation turned off.

@BenWibking
Copy link
Collaborator

Yes, I agree. Feel free to add a PR with this check and an informative error message.

@BenWibking BenWibking added bug: usability and removed bug: wrong answer/failure/crash Something isn't working labels Dec 22, 2024
@BenWibking
Copy link
Collaborator

BenWibking commented Dec 22, 2024

Since it doesn't actually cause wrong results or a failure in a case where it should work, I'm going to take off the red 'bug' label. But it should be more informative to the user, so I've instead added the yellow 'bug: usability' label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants