Use atomic data for lanthanide ions V-VII from Carvajal-Gallego et al. (MONS university) by ccollins22 · Pull Request #4 · artis-mcrt/artisatomic

ccollins22 · 2024-07-19T15:41:52Z

No description provided.

…ent with NIST

lukeshingles

A few minor style issues, but what's really important is to add a test for the new functionality to avoid it getting broken by future changes. If the full dataset is too large, you can split out some representative data and test that. If it's under ~5MB, you can commit it directly to the repo.

lukeshingles · 2024-07-22T09:37:58Z

artisatomic/__init__.py

+    if levela is None or levela.parity is None or levelb.parity is None:
+        return False


When would levela be None?

Because of energy_levels = [None]
Which I copied from your code to read Tanaka's data. Without initialising the array with None the lowest level doesn't get written out. With it levela doesn't have attribute parity and this fails

Ok, that's a relic of our 1-based indexing that we inherited from the existing artis file formats. However, if any of your transitions are referencing the dummy level in the 0 index, that suggests that there is an off-by-one error in the indexing of transitions.

Should the transitions array have a dummy value initialised too then?

No, we don't really keep track of a transition index, and all transitions get written out in write_transition_data(). write_adata() skips the 0-index when writing out the list of levels for each ion.

It's important to manually verify that the first couple of levels and transitions are recorded correctly in the artis output files, because indexing errors are very easy to make.

One file with the transitions is ~30MB

Just take a sample of a few hundred transitions and compress it with zstd.

lukeshingles · 2024-07-22T09:42:46Z

artisatomic/__init__.py

This is better as far as not repeating the forbidden conditions in several places, but I'm not sure we want to pay the cost of two more Python function calls on each one of ~50 million transition. Did this affect performance in any significant way?

I think a better way (but can be left for a separate PR) would be to use a forbidden attribute on each transition.

I didn't notice a big slow down but I didn't really check. Feel free to update

lukeshingles · 2024-07-22T09:43:28Z

artisatomic/__init__.py

artisatomic/readmonsdata.py

lukeshingles · 2024-07-22T09:59:03Z

artisatomic/readmonsdata.py

It's not a blocker, but I would strongly prefer to avoid using pandas in any new code (so that eventually we can remove the import and save ~500ms of startup time). Polars should be a drop-in replacement here.

Feel free to update - I'm not used to polars yet

I suggest getting the test in place first, and then these style fixes can be implemented safely with a big warning if anything breaks.

artisatomic/readmonsdata.py

lukeshingles · 2024-07-22T10:05:47Z

artisatomic/readmonsdata.py

Why use an iterator for lower and upper but then access transition_wavelength_A and oscillator_strength by index? Wouldn't it be better to zip all of these arrays? Shouldn't strict be True here?

I didn't add strict - I've no idea what it does. I think it was the automated checks?

I didn't add strict - I've no idea what it does. I think it was the automated checks?

Ruff modified it.

Hmm, that's a bit of an annoying autofix. Strict=False will stop iterating at the shortest of any arrays, while strict=True will raise an exception if the lengths don't match. I think we almost always want strict=True.

lukeshingles · 2024-07-22T10:08:54Z

artisatomic/readmonsdata.py

if you're not using the lowerlevel value from enumerate, why use enumerate? Better to zip lowerlevel, upperlevel, A_ul with strict=True to make sure they match lengths.

lukeshingles · 2024-07-22T10:10:24Z

atomic-data-mons/url-lanthanidesV-VII.txt

@@ -0,0 +1,4 @@
+https://doi.org/10.5281/zenodo.10635803


Please add direct URLs for the zip files that are needed as we do for other data sources.

I'm not sure what the links are. Does it work to just add the files names to the end of the zenodo link?

https://zenodo.org/records/10635803/files/outggf_Ln_V--VII.zip
https://zenodo.org/records/10635803/files/outglv_Ln_V--VII.zip
The readme should probably just be committed directly to the repo since it's so small.

The info from the readme is in comments at the top of the file

lukeshingles · 2025-07-04T10:45:51Z

artisatomic/readmonsdata.py

+from astropy import constants as const
+from astropy import units as u


Adding another dependency (astropy) to the package is not really necessary. hc_in_ev_cm and hc_in_ev_angstrom can be set with numeric literals.

ccollins22 added 16 commits July 4, 2024 13:12

Create url-lanthanidesV-VII.txt

15b5dce

Read mons lanthanide data

547d81f

Add condition for None parity (forbidden False)

99df426

Add handler for MONS data

bea1556

Read all files and tidy code

23d076c

Add comment to URL file

63b88c0

Update assert for rounding issue (NIST ionisation energy only to 1dp)

f70235d

Update log message

c417d70

Update forbidden check

e45872c

Fix lowest level

c9afac4

Use ionization energy from MONS data but check to make sure in agreem…

a979411

…ent with NIST

Remove commented out code

98d7381

Get ion_handlers for all mons data

f18892f

Use NIST ionisation potentials where MONS data does not match

da539df

Update readmonsdata.py

a81fa62

Update __init__.py

c02adb0

ccollins22 requested a review from lukeshingles July 19, 2024 15:45

lukeshingles requested changes Jul 22, 2024

View reviewed changes

ccollins22 and others added 9 commits July 22, 2024 12:25

Update readmonsdata.py

7fafef7

Update readmonsdata.py

af7a971

Update readmonsdata.py

e0c3a59

Update readmonsdata.py

113ae6d

Update readmonsdata.py

28919d2

Update readmonsdata.py

34c8da4

Fix level indicies, A values, and transition counts

376ab51

Remove debug print

5187e3c

Simplify code

ea88c3c

lukeshingles force-pushed the main branch from 8bf046c to fa24de4 Compare April 17, 2025 08:55

lukeshingles force-pushed the main branch from 1491102 to 4240e60 Compare May 28, 2025 13:12

lukeshingles reviewed Jul 4, 2025

View reviewed changes

lukeshingles force-pushed the main branch 2 times, most recently from 4a34a7e to aa59f68 Compare September 3, 2025 13:40

lukeshingles force-pushed the main branch 4 times, most recently from 610a929 to fa9465b Compare January 28, 2026 16:42

		if levela is None or levela.parity is None or levelb.parity is None:
		return False

		from astropy import constants as const
		from astropy import units as u

Conversation

ccollins22 commented Jul 19, 2024

Uh oh!

lukeshingles left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments