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

Revive Relativistic Boris Push #2708

Merged
merged 54 commits into from
Jun 27, 2024

Conversation

JaydenR2305
Copy link
Member

@JaydenR2305 JaydenR2305 commented May 31, 2024

Reimplements the relativistic Boris push integrator defined in #979 using the new ParticleTracker framework

Must be merged after #2704

@JaydenR2305 JaydenR2305 requested review from namurphy, pheuer and a team as code owners May 31, 2024 17:09
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

Thank you for submitting a pull request (PR) to PlasmaPy! ✨ The future of the project depends on contributors like you, so we deeply appreciate it! 🌱

Our contributor guide has information on:

Important

PlasmaPy recently switched to an src layout. The source code previously in plasmapy/ is now in src/plasmapy/. Tests are now in tests/. If you have previously done an editable installation, it will likely need to be re-done (i.e., with pip install -e .[tests,docs] in the top-level directory of the repository). The former plasmapy/ directory will need to be deleted manually in old clones because git does not track directories.

The bottom of this page shows several checks that are run for every PR. Don't worry if something broke! We break stuff all the time. 😺 Click on "Details" to learn why a check didn't pass. Please also feel free to ask for help. We do that all the time as well. 🌸 You can find us in our chat room or weekly community meeting & office hours. Here are some tips:

  • Try fixing CI / Python 3.12 test failures first.
  • Most pre-commit.ci - pr failures can be automagically fixed by commenting pre-commit.ci autofix below, followed by a git pull to bring the changes back to your computer. Please also see our pre-commit troubleshooting guide.
  • If pre-commit.ci - pr says that a function is too long or complex, try breaking up that function into multiple short functions that each do one thing. See also these tips on writing clean scientific software.
  • If the CI / Documentation check ends with a cryptic error message, check out our documentation troubleshooting guide.
  • For a documentation preview, click on Details next to docs/readthedocs.org:plasmapy.

If this PR is marked as ready for review, someone should stop by to provide a code review and offer suggestions soon. ✅ If you don't get a review within a few days, please feel free to send us a reminder.

Please also use SI units within PlasmaPy, except when there is strong justification otherwise or in some examples.

We thank you once again!

@JaydenR2305
Copy link
Member Author

JaydenR2305 commented Jun 17, 2024

figured it out! There was some dependence on the initial velocity. I've set the initial velocity to zero for now until we can figure out a better alternative (setting it directly to some fraction of the final drift velocity wasn't working...) This just means that we have to discard the first 20 points or so to allow the particle to reach the expected drift velocity.

edit: disregard, this is false. I was looking at plots of the relative errors

@pheuer
Copy link
Member

pheuer commented Jun 17, 2024

figured it out! There was some dependence on the initial velocity. I've set the initial velocity to zero for now until we can figure out a better alternative (setting it directly to some fraction of the final drift velocity wasn't working...) This just means that we have to discard the first 20 points or so to allow the particle to reach the expected drift velocity.

Awesome! Kind of concerning it doesn't work with an initial velocity thought... any idea why that is? Want to make sure that's not an issue with the analytic solution or the pusher or something.

@JaydenR2305
Copy link
Member Author

JaydenR2305 commented Jun 17, 2024

Awesome! Kind of concerning it doesn't work with an initial velocity thought... any idea why that is? Want to make sure that's not an issue with the analytic solution or the pusher or something.

image
I think the derivation makes the assumption that the particle is starting from rest (taken from https://journals.aps.org/pre/pdf/10.1103/PhysRevE.72.026603)

Though it looks like it would be as simple as picking up a phase difference (plus the amplitude difference picked up by gamma)?
image

@pheuer
Copy link
Member

pheuer commented Jun 17, 2024

I think the derivation makes the assumption that the particle is starting from rest

Ah, got it, ok that's expected then! In that case I'd say this test looks good enough?

Copy link
Member

@pheuer pheuer left a comment

Choose a reason for hiding this comment

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

Looking good! Few minor comments before we merge

src/plasmapy/simulation/particle_integrators.py Outdated Show resolved Hide resolved
tests/simulation/test_particle_tracker.py Outdated Show resolved Hide resolved
tests/simulation/test_particle_tracker.py Outdated Show resolved Hide resolved
tests/simulation/test_particle_tracker.py Outdated Show resolved Hide resolved
@JaydenR2305 JaydenR2305 marked this pull request as ready for review June 25, 2024 16:12
Copy link
Member

@pheuer pheuer left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for polishing off this neglected old PR!!

@pheuer pheuer merged commit 263bf74 into PlasmaPy:main Jun 27, 2024
17 checks passed
@JaydenR2305 JaydenR2305 deleted the relativistic-boris-push branch June 27, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Related to continuous integration docs PlasmaPy Docs at http:https://docs.plasmapy.org documentation infrastructure notebooks Related to example Jupyter notebooks in docs/examples/ plasmapy.diagnostics Related to the plasmapy.diagnostics subpackage plasmapy.simulation Related to the plasmapy.simulation subpackage python Pull requests that update Python code static type checking testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants