-
Notifications
You must be signed in to change notification settings - Fork 335
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
Enable particle_stepper to run under CI #668
Enable particle_stepper to run under CI #668
Conversation
Codecov Report
@@ Coverage Diff @@
## master #668 +/- ##
==========================================
+ Coverage 94.95% 94.95% +<.01%
==========================================
Files 54 55 +1
Lines 4656 4657 +1
==========================================
+ Hits 4421 4422 +1
Misses 235 235
Continue to review full report at Codecov.
|
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.
Looks mostly good! I have two suggestions related to naming and the choice of time step.
|
||
species = Species(plasma, 'p', 1, 1, 1e-10 * u.s, 10000) | ||
particle = ParticleTracker(plasma, 'p', 1, 1, 1e-4 * u.s, 40) |
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.
The naming here may be a bit confusing, given plasmapy.atomic.Particle
. Perhaps rename this as particle_trajectory
or tracked_particle
or something like that?
@@ -32,39 +33,38 @@ | |||
|
|||
############################################################ | |||
# Initialize the particle. We'll take one proton `p` with a timestep of | |||
# $10^{-10}s$ and run it for 10000 iterations. | |||
# $10^{-4}s$ and run it for 40 iterations. |
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.
I was looking at the giles preview of this example, and it appears that the trajectory is more or less a straight line, with some bumpiness in the z coordinate. It'd be good to choose the timestep to be exactly one gyroorbit.
Hello @StanczakDominik! Thanks for updating your pull request. Congratulations! There are no PEP8 issues in this pull request. 😸 Comment last updated at 2019-09-03 18:08:56 UTC |
This should look better now! |
I went back to |
As pointed out by @diego7319, this example wasn't running in CI and wasn't being actively tested. I simplified it a bit to run faster along the way.