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
Added relativistic boris push #979
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions, and we do have to figure out how to test this, because the implementation looks correct enough to fool me 😄 I did start reworking the particletracker tests in #675, and I'll prioritize bringing them in for now. To get this merged before I do, I see two options atm:
- Hack together some tests like, "two trajectories basically equivalent at low initial velocities, to floating point accuracy", "two trajectories completely different according to some heuristic such as end position/velocity at high initial velocities"
- Wait until we get some better ideas and move this into
_wip_integrators
inparticletracker
; also add a warning at https://github.com/pheuer/PlasmaPy/blob/relativistic_boris_push/plasmapy/simulation/particletracker.py#L113if integrator not in self.integrators
that this one is untested for now. Possibly add afilterwarnings
in your code to ignore that.
Whatcha think?
# Birdsall has a factor of c incorrect in the definiton of t? | ||
# See this source: https://www.sciencedirect.com/science/article/pii/S163107211400148X | ||
t = q * B * dt / (2 * γ1 * m) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: look into this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please do - I'm very skeptical whenever my math disagrees with the textbook!
Co-authored-by: Dominik Stańczak <stanczakdominik@gmail.com>
Co-authored-by: Dominik Stańczak <stanczakdominik@gmail.com>
@StanczakDominik I like option 2: saves us from writing tests now that will just be replaced with better ones soon. I've added a warning like you suggested - what do you think? |
Co-authored-by: Dominik Stańczak <stanczakdominik@gmail.com>
Sounds good! I'll dig into that paper as soon as I'm able, and we can probably merge it. |
@StanczakDominik Thanks - I think I was just looking at the definitions in section 2.2.1 of that paper (although maybe the Lorentz-invariant formulation they talk about later is worth looking into). |
Okay, change of plans due to telecon where I was reaffirmed in thinking that having this go in untested is a meh idea; I'll add them.
|
And if you wanna continue building on top of this, just do so and we'll do git magic to account for it :) |
Codecov Report
@@ Coverage Diff @@
## master #979 +/- ##
==========================================
- Coverage 96.86% 96.07% -0.80%
==========================================
Files 70 70
Lines 6864 6881 +17
==========================================
- Hits 6649 6611 -38
- Misses 215 270 +55
Continue to review full report at Codecov.
|
So this is pretty wack. I'm pretty sure I said somewhere that Looks like I lied! The tests are failing now, and the way I've got them fixed locally is actually using This means we've got a pretty simple speed optimization available before us; but it's out of scope of this PR, so let's get this one merged faster. I'll continue reworking the particle tracker and optimize that out. |
I just want to restate how confused I am right now. |
Okay, I figured it might be a good idea to still test whether the relativistic case outputs correct results. Doesn't look like it, based on the notebook. And since the particletracker code is a bit of a dumpster fire right now (with myself, of course, to git blame), I think it makes more sense to come back to this once I've got that cleaned up. |
Well, admittedly I don't even know good test cases for the relativistic pusher... |
I agree, we can come back to this a bit later. What test did you run on the relativistic pusher (the notebook you mentioned earlier)? One option would of course be making sure that it matches the non-relativistic Boris push for non-relativistic particles. Another might be pushing a relativistic particle a single time step in a known uniform field and somehow analytically calculating what the right answer should be another way to compare? |
I did run the non-relativistic limit as a test, and the results agree; but I'd feel bad having a relativistic pusher go in without testing it in the relativistic limit... And I have little subject matter expertise on the relativistic limit, so, yeah. |
@StanczakDominik So I'm not sure how to best integrate this into In the following plots I'm plotting the coordinate in the direction of the drift (parallel to E). If E=0.05 * cB, the ExB drift is effectively non-relativistic: But if E = 0.5 * cB, there is a clear relativistic effect I'm not entirely sure why vd=E/B matches the relativistic prediction here...I thought that was the classical ExB drift? Would it be difficult to implement a test that runs Here's the code:
|
Oh hell yeah, that's a great test! It shouldn't be too hard; stick an L2 norm on the difference between the two trajectories and it should (hopefully...) disappear (or at least, become very smol) for small timesteps. I can try handling that after Friday. |
Added relativistic Boris push to
simulation.particle_integrators
. This is necessary for the synthetic proton radiography module (protons are typically > 1% c).I think I wrote it correctly (based on Birdsall), but it would be great if someone could check my work.
Also, currently there are no tests. Ideally this should be tested as part of the particletracker tests (maybe showing that it produces the same result in the non-relativistic limit as the regular Boris push) but those tests are currently in flux.