Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the total ion density to BeamCXPEC in BeamCXLine and add donor_metastable attribute to BeamCXPEC #444

Merged
merged 6 commits into from
Jul 31, 2024

Conversation

vsnever
Copy link
Member

@vsnever vsnever commented Jun 29, 2024

This PR fixes #441 and #411.

  • The docstrings for the core and the openadas BeamCXPEC classes are updated to reflect that the total ion density rather than the receiver ion density is needed to evaluate the effective CX emission coefficient.
  • The BeamCXLine now passes the total ion density to the BeamCXPEC.evaluate() method instead of the receiver ion density.
  • The donor_metastable attribute is added to the core BeamCXPEC class.

I've added a warning to the changelog that this fix affects user results. The affected figures in the documentation have also been replaced.

Please note that tests for Python 3.9 and 3.10 will fail due to #442.

Copy link
Member

@Mateasek Mateasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vsnever,

thanks for a quick fix. I had only one comment I would like you to consider. The second one is only a minor picky suggestion.

As always, great job!

cherab/core/atomic/rates.pyx Show resolved Hide resolved
cherab/core/model/beam/charge_exchange.pyx Outdated Show resolved Hide resolved
@vsnever
Copy link
Member Author

vsnever commented Jul 10, 2024

Hi @Mateasek,

Thanks for review. In the comment I tried to explain why I added the donor_metastable attribute to the base BeamCXPEC and why I included the fixes for #411 and #441 in the same PR. But if you are not happy with that, I can move a fix for #411 in a separate PR.

Copy link
Member

@Mateasek Mateasek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. This is good to be merged

@jacklovell jacklovell merged commit 7552b72 into cherab:development Jul 31, 2024
8 checks passed
@vsnever vsnever deleted the bugfix/beamcxpec branch August 30, 2024 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants