Closed reichherzerp closed 2 years ago
Looks good to me - might be good practice to work the code from the bug report into a test.
Thanks @TobiasWinchen - I added tests that govern the bug report and check the new magnetic read functionality in the propagation modules (CK and SDE).
Nice!
@reichherzerp nice work. I only have a few things in mind:
Also the getField/getFieldAtPosition methods are not an unambiguous method name for the DiffusionSDE module as this propagation module can have two different fields (magnetic for the diffusion directions and another one for the advection).
If I remember correctly the gas and photon density modules could also be improved with the PR.
Thanks for the feedback. I also think that your suggestions fit well into the framework of the current PR, which is why I implemented the proposed enhancements. I have then also tested the new cross-language polymorphism features and all tests pass. Is it like you had it in mind?
Looks good to me now. I will install this version locally and run the tests also on my system. I think as long as the automated tests are not back online it's good to have at least some people run the tests to not only rely on a single system setting.
This PR fixes #365. Currently, CRPropa can't handle custom magnetic fields implemented in Python. This PR allows for custom magnetic fields via Python.
Background To correctly implement a Python class that inherits from the wrapped C++ MagneticField class that contains virtual methods, cross-language polymorphism via directors is needed. The child Python class for the custom magnetic field should override the getField base class function. However, as we don't have enabled directors for the MagneticField class, this fails currently.
Changes Inserting the decorator in the
2_headers.i
file solves the issue. For enhanced testability and synchronization between the propagation modules, I introduced getField functions for PropagationCK and DiffsuionSDE, similar to PropagationBP. I also added better handling of possible magnetic field issues during the propagation.