Refactoring of ParticleTracker, Plasma class #675
Conversation
Codecov Report
@@ Coverage Diff @@
## master #675 +/- ##
==========================================
- Coverage 95.87% 94.61% -1.26%
==========================================
Files 56 59 +3
Lines 5113 5353 +240
==========================================
+ Hits 4902 5065 +163
- Misses 211 288 +77
Continue to review full report at Codecov.
|
|
Hello @StanczakDominik! Thanks for updating your pull request.
Comment last updated at 2020-04-30 13:05:13 UTC |
Getting an issue with setters, maybe?
There was a problem hiding this comment.
This has been my most productive bus trip every! The wifi is blocking ssh traffic, though, and I'm a bit bored of copypasting a personal access token to the password field, so I'll keep it as it is for now.
This is a first draft and I would really appreciate suggestions!
This is also sort of how I would imagine our numba based simulations and units usage for interfaces should look like, ideologically. Please take a look at this design pattern and see if you can optimize it (especially the multiple @propertyies, those are a pain).
| @@ -0,0 +1,43 @@ | |||
| """ | |||
There was a problem hiding this comment.
This file is very much so a "get something working" quick and dirty copypaste of plasmablob that I'll be cleaning up later. I'm simply wondering, though, if this kind of functionality is better plugged into plasmablob or left here, as a generic "plug in your own functions of space" thing.
There was a problem hiding this comment.
That's a good question; maybe we should talk about how to organize. It would be very useful functionality to have a class that we can use to represent plasma configurations analytically.
| # atol=estimated_gyrofrequency_std), \ | ||
| # "Gyrofrequencies don't match!" | ||
| s = ParticleTracker(test_plasma, particle_type=particle_type, dt=dt, nt=int(1e4)) | ||
| s._v[:, 1] = perp_speed.si.value |
There was a problem hiding this comment.
I had some issues with directly modifying s.v[:, 1] = perp_speed, still figuring that one out
There was a problem hiding this comment.
oh crud, https://stackoverflow.com/questions/3137685/using-property-decorator-on-dicts I think this is gonna be basically impossible
| @@ -0,0 +1,43 @@ | |||
| """ | |||
There was a problem hiding this comment.
That's a good question; maybe we should talk about how to organize. It would be very useful functionality to have a class that we can use to represent plasma configurations analytically.
| from plasmapy.atomic import atomic | ||
| from astropy import constants | ||
| from astropy import units as u | ||
| import numba | ||
| from plasmapy.utils import PhysicsError | ||
| import tqdm |
There was a problem hiding this comment.
Nice! I hadn't heard of tqdm, so I'll have to look into it.
| dtype=float) | ||
| self._velocity_history = np.zeros((self.NT, *self.v.shape), | ||
| dtype=float) | ||
| self._hqmdt = (self.eff_q / self.eff_m / 2 * dt).si.value |
There was a problem hiding this comment.
I am wondering if there is a better variable name than hqmdt.
There was a problem hiding this comment.
There most certainly is. But if I wrote "half charge timestep over mass", line length would explode in a few places.
On the other hand, I have been reading Computer Simulation Using Particles recently, so my thinking may be contaminated by the FORTRAN examples...
|
I'm giving up on running calculations on |
… into RefactorTracker
Co-authored-by: Erik Everson <eteverson@gmail.com>
Co-authored-by: Erik Everson <eteverson@gmail.com>
The tox environment does not use `make html-nonb` because tox envs, by default, don't have `make`! We call the sphinx command directly instead. The Makefile was also broken. It's now fixed.
|
Astropy.units might work better with numba now, so some refactoring and simplification should be possible -> back to drafting board! |
|
numba/numba#4637 (comment) would be the thing to try out when coming back to this PR. However... I think this has become overly bloated and basically unmergeable. I think the way to go may be to close this one down and break it up in a few smaller, separate PRs, redoing the changes. I still think it was worthwhile, because I now know what to do for this. @rocco8773, whatcha think? |
|
@StanczakDominik I think that's a good idea. I agree with all of that. |
|
RIP this one, then! |
|
This pull request has been mentioned on PlasmaPy Community Discourse Forum. There might be relevant details there: |
namurphy
added
plasmapy.simulation
Closes #636
This is going to be a pretty open-ended one. I've got a few ideas I'd like to tinker with:
plasma3dwith draft AnalyticalPlasma in ParticleTracker, which probably shares a whole bunch of functionality with PlasmaBlobLeftovers for future work: