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

Develop for dielectronic recombination #161

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Develop for dielectronic recombination #161

wants to merge 4 commits into from

Conversation

ssim
Copy link
Contributor

@ssim ssim commented Dec 5, 2024

DO NOT MERGE - WIP!

This is for implementing dielectronic recombination into ARTIS.

Current status (5th Dec 2024):

  • Draft infrastructure to store autoionization data for levels.
  • Autoionization and capture into resonances implemented into nlte population solver
  • Preliminary testing for Si ions in 1 zone model appears to run and give roughly correct effect
  • Pushing to GitHub to keep a safe copy! But this is not in a state yet to actually merge (so please don't anyone do that!)
  • File attached here is the format proposed for supplying a list of autoionization processes. Format is pretty simple: list of Z upper_ion upper_level lower_ion lower_level A_auto

Still to do (5th Dec 2024):

  • Need to figure out how to handle with superlevels.
  • Likely don't actually want the autoionizing levels to take up space in the stored populations (i.e. should probably include them in the solver, but not include them in the stores). Not yet implemented.
  • Lower priority: need to implement into the macro atom (that likely required some more infrastructure stuff added)
  • Lower priority still: need to implement into the heating/cooling
  • Need to properly test

NOTE: I think that the superevel renorm factors currently in the code may be wrong for ionization processes. Need to check - but should discuss with Luke and see if we can fix. Don't think that actually will affect anything, but we should either fix or remove and abort.

autoion.txt

@ssim ssim requested a review from lukeshingles as a code owner December 5, 2024 17:58
@ssim ssim marked this pull request as draft December 5, 2024 18:05
@lukeshingles
Copy link
Member

The code generally looks good to me (including matching the style of the surrounding code!). Possibly I missed something, but it looks like you could avoid the two-pass reading of the autoion.txt and just append to the vector as lines are read. Something like:

globals::elements[element].ions[lowerion].levels[lowerlevel].allautoion_startdown = temp_allautoion.size();
temp_allautoion.push_back({.autoion_A = autoion_A, .elementindex = element, ...});

The nice thing about vectors is that they autoresize as items are appended. I only avoid doing that for very large data structures, because the allocated size can be bigger than the real size by up to a a factor of two. Since the vector is probably going to be fairly small, and it will be copied into precisely-sized shared memory before being erased anyway, that shouldn't be a problem.

clang-tidy is telling you that a couple of variables can be marked as constants.

When autoion.txt is ready, you can commit a copy in the repository's data folder and we can make a test case to prevent breakage. The autoion.txt file would be slightly easier/faster to read in python if it followed single space delimiters instead of fixed width format, but if the file size is going to be small, this is not too important. Fixed width is also easier for hand editing.

(It's not possible to merge a draft pull request, so no risk of that!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants