Uf23 #490
Uf23 #490
Conversation
|
Hi @MichaelUnger |
|
Hi @lukasmerten Cheers, |
| */ | ||
| UF23Field(const ModelType mt); | ||
| /// no default constructor | ||
| UF23Field() = delete; |
There was a problem hiding this comment.
You probably chose to not provide a default model because none of the models yielded a significantly better fit than the others, right?
There was a problem hiding this comment.
Yes, I think that not providing a default model makes the user aware that this is a collection of 8 models to choose from.
| double& fPoloidalR = fParameters[ePoloidalR]; | ||
| double& fPoloidalW = fParameters[ePoloidalW]; | ||
| double& fPoloidalZ = fParameters[ePoloidalZ]; | ||
| double& fPoloidalXi = fParameters[ePoloidalXi]; |
| } | ||
|
|
||
| namespace crpropa { | ||
| UF23Field::UF23Field(const ModelType mt) : |
There was a problem hiding this comment.
Could you add a comment that the UF23 model is internally using 1e-6*gauss=1, kpc=1, etc.
If you have a better place to make this statement I am also fine with it.
There was a problem hiding this comment.
Added a comment it at this point of cpp file, since it is internal to the code and not visible to the outside user.
|
Please add a comment to the CHANGELOG file under new features. |
|
Apart from the small changes I suggested this PR looks good to me. |
|
Thanks for the review, I changed the fork accordingly. |
added new models of the coherent magnetic field from 2311.12120