Closed vsnever closed 3 months ago
Thanks for the reviews, @Mateasek, @jacklovell.
Since the beam direction became more complex, I think it would be nice to add the equations you use into the documentation. I think it would be enough if you add the equations and plot you posted in the opening post of this PR. It would be explanatory enough and would remove the necessity to go through the cython code for the users, what do you think?
I added equations for the beam density and direction in SIngleRayAttenuator.density()
and Beam.direction()
docstrings respectively. I noticed that in the docs, the description of the SingleRayAttenuator
class was placed under the "CXS models" section, so I moved it to a separate "Beam attenuation" section. The beam direction plot is added to the particle_beams.rst
.
I'd also suggest adding some emphasis (* or ) to the line in the changelog highlighting that this will change existing results.
I agree, the line announcing this change in the changelog is now in bold.
If there aren't already any unit test associated with this, now is also a good time to add them. But don't worry if that's too big a project: I wouldn't want to hold up an otherwise useful change because of the work involved in setting up a mock beam for tests.
We already have tests for the beam CX model, so setting up a mock beam and plasma is not a problem. I added the tests for the beam density and direction. They test these values on-axis, off-axis and outside the beam.
Looks good. Couple of minor suggestions.
Thanks, @jacklovell. All done.
This updates density and direction calculation of the neutral beam model and fixes #414.
The broadening of the BES line shape is now naturally determined by the beam divergence. Therefore, setting beam temperature to the physically unreasonable values is no longer required. The parameters of the beam in the BES demo are updated.
Although this change does impact user results, I think it is justified by the improved consistency of the beam model.