Implementation of the new PolarizedSingleModeMagneticField class

[asavelie]

This contribution adds the possibility of defining a magnetic field with a single polarization mode in CRPropa. This includes the more specific circular and linear polarizations, but also the more general elliptical one, as well as a formalism to handle magnetic helicity.

[rafaelab]

Apart for conflicts in CMakeLists.txt, I believe this pull request is almost ready. A notebook with an example is the only thing missing.

[adundovi]

@asavelie How do we test it quickly? What is the simplest known result one should obtain from this?

[asavelie]

@asavelie How do we test it quickly? What is the simplest known result one should obtain from this?

Well, since there is an analytical form of the magnetic fields, I would say that the simplest test should be to plot a vector plot or a field line plot of the magnetic field for different parameter combinations.

[rafaelab]

@asavelie How do we test it quickly? What is the simplest known result one should obtain from this?

Well, since there is an analytical form of the magnetic fields, I would say that the simplest test should be to plot a vector plot or a field line plot of the magnetic field for different parameter combinations.

Since the fields can be analytically obtained, maybe simple sanity checks can be added to the 'tests/' folder?

[rafaelab]

@asavelie How do we test it quickly? What is the simplest known result one should obtain from this?

Well, since there is an analytical form of the magnetic fields, I would say that the simplest test should be to plot a vector plot or a field line plot of the magnetic field for different parameter combinations.

Since the fields can be analytically obtained, maybe simple sanity checks can be added to the 'tests/' folder?

Hi @asavelie Could you just add some quick sanity checks to testMagneticField.cpp? If this module has to be maintained in the future the tests will help us make sure everything is working properly.

[rafaelab]

Hi @asavelie

This is a much simpler version now!

I have just one (rather important) concern. The function getField is called at every step. The shorter it is, the faster the code runs. A lot of unnecessary checks are being done with all the if/else's. Maybe there could be one getField() for each scenario. For instance:

class LinearlyPolarizedMagneticField: public PolarizedMagneticField {

LinearlyPolarizedMagneticField(const double &B_0, const double &wavelength, const double &sigma, const Vector3d &r_0, const Vector3d &e_1, const Vector3d &e_2, std::string flagAmplitudeRms) :
            B_0(B_0), wavelength(wavelength), sigma(sigma), r_0(r_0), e_1(e_1), e_2(e_2), flagAmplitudeRms(flagAmplitudeRms) {
    }

Vector3d getField(const Vector3d &position) const;

In this implementation, there would be something like a class CircularlyPolarizedMagneticField and a EllipticallyPolarizedMagneticField. In terms of work, this would mean just to separate the current getField function in a few others to remove all the if-statements.

[rafaelab]

@asavelie I forgot to say: could you also update the CHANGELOG file?

[asavelie]

@asavelie How do we test it quickly? What is the simplest known result one should obtain from this?

I implemented a test in the MagneticField test source file.

Hi @asavelie

This is a much simpler version now!

I have just one (rather important) concern. The function getField is called at every step. The shorter it is, the faster the code runs. A lot of unnecessary checks are being done with all the if/else's. Maybe there could be one getField() for each scenario. For instance:

class LinearlyPolarizedMagneticField: public PolarizedMagneticField {

LinearlyPolarizedMagneticField(const double &B_0, const double &wavelength, const double &sigma, const Vector3d &r_0, const Vector3d &e_1, const Vector3d &e_2, std::string flagAmplitudeRms) :
          B_0(B_0), wavelength(wavelength), sigma(sigma), r_0(r_0), e_1(e_1), e_2(e_2), flagAmplitudeRms(flagAmplitudeRms) {
  }

Vector3d getField(const Vector3d &position) const;

In this implementation, there would be something like a class CircularlyPolarizedMagneticField and a EllipticallyPolarizedMagneticField. In terms of work, this would mean just to separate the current getField function in a few others to remove all the if-statements.

Done! Now the if statements are only called once in the beginning.

@asavelie I forgot to say: could you also update the CHANGELOG file?

Done!

[rafaelab]

Hi @asavelie Now it is read to be merged. But there are some conflicts. You probably should fetch the upstream version of the repository to sync your branch with the main CRPropa repo. Once that is done, I'll approve and merge the PR immediately.

[rafaelab]

@asavelie I'm having trouble compiling it. You probably synced your branch with the upstream but not CMakeLists.txt. I believe this issue is fixed by removing the line "src/GridTurbulence.cpp", since the file no longer exists. If you are doing that, maybe you can also make sure to use the same indentation for everything (src/module/Redshift.cpp is aligned left; your own PolarizedSingleMode... and TF17 also). After that I will merge your PR.