Fix duplicate entries in continuum line list #2443

AlexHls · 2023-10-12T13:00:24Z

📝 Description

Type: 🪲 bugfix

Attemts to fixe #2442

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

tardis-bot · 2023-10-12T13:01:01Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

wkerzendorf · 2023-10-12T13:03:30Z

tardis/plasma/properties/transition_probabilities.py

+        try:
+            fb_cooling_probs.insert(
+                0,
+                "transition_type",


but these are different - right? one inserts value[0] the other inserts the value. Why are there sometimes multiple? are they duplicate? Should we drop them?

This bit is just a workaround to avoid an error from this. This should (probably) be removed before merging. I suspect that this is only a symptom of the issue. The duplicates are already present in atomic.py.

tardis-bot · 2023-10-12T13:07:33Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

AlexHls · 2023-10-12T13:08:35Z

tardis/plasma/properties/atomic.py

@@ -467,6 +467,14 @@ def calculate(self, atomic_data):
        level_idxs2line_idx = pd.Series(
            np.arange(len(index)), index=index, name="lines_idx"
        )
+
+        # Check for duplicate indices


We already get duplicate indices here, completely unrelated to the actual continuum lines. For now I added a ValueError to catch this early, but I'm not sure if we can simply drop the duplicate indices.

tardis-bot · 2023-10-12T18:27:11Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

tardis-bot · 2023-10-12T18:29:07Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

wkerzendorf

Please follow the TARDIS bot instructions

codecov · 2023-10-12T18:41:11Z

Codecov Report

Merging #2443 (5e17989) into master (ecd011c) will decrease coverage by 0.01%.
The diff coverage is 33.33%.

❗ Current head 5e17989 differs from pull request most recent head d7ce1e7. Consider uploading reports for the commit d7ce1e7 to get more accurate results

@@            Coverage Diff             @@
##           master    #2443      +/-   ##
==========================================
- Coverage   66.91%   66.91%   -0.01%     
==========================================
  Files         153      153              
  Lines       13633    13636       +3     
==========================================
+ Hits         9123     9124       +1     
- Misses       4510     4512       +2

Files	Coverage Δ
tardis/plasma/properties/atomic.py	`85.60% <33.33%> (-0.62%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

review-notebook-app · 2023-10-12T18:45:09Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

* Allow Simulation Tests to Run Independently (tardis-sn#2407) * Modify test_packet_source.py so that sim and tardis/tests can run * remove BasePacketSource * format * Add docstrings * Pre-release 2023.09.17 (tardis-sn#2411) Automated changes for pre-release 2023.09.17 * upgrading env in 2022 (tardis-sn#2410) * fix several bugs related to upgrade * fix the get function in pandas * fixed several incompatibilities for new libraries * fixed up the changes in pandas testing and other reindexing issues * fix other bugs with new environment * blackify tardis * updated the conda-lock file * add lock files * update cache number * fix simulation tests * remove units for testing * blackify * Update tardis/visualization/tools/tests/test_sdec_plot.py good catch Co-authored-by: Atharva Arya <[email protected]> * add assert * reset caches --------- Co-authored-by: Atharva Arya <[email protected]> * In gamma ray code : Changed times to np.geomspace (tardis-sn#2415) * Changed times to np.geomspace * Added myself to mailmap * rename RadialModel1D to SimulationState (tardis-sn#2417) * rename RadialModel1D to SimulationState * black files * rename simulation_state2 (tardis-sn#2421) * fixup of many files to make them pass the tests * change model for typo * Ignore tests in docstr coverage (tardis-sn#2424) ignore tests in docstr coverage * Disabled formal integral for continuum interaction (tardis-sn#2426) Disabled formal integral Co-authored-by: Alexander Holas <alexander.holas@h-its> * add linelist exposure to atom data and reader, built on current (tardis-sn#2428) * add linelist exposure to atom data and reader, built on current * add josh to mailmap * Modified how setup input energy takes isotope name. Earlier it was ga… (tardis-sn#2425) Modified how setup input energy takes isotope name. Earlier it was gamma_ray_lines.Isotope * Added a function to calculate shell masses (tardis-sn#2434) * Added a function to calculate shell masses * Added a function to calculate shell masses * changed shell masses with the new function * Fix duplicate entries in continuum line list (tardis-sn#2443) * Workaround list access * Formatting * Added error if duplicate * Remove duplicates * Fix typo, add comment * Remove bandaid fix * Cleanup * More cleanup * Fix numba parallel issues (tardis-sn#2447) * Enabled parallel * Fix the mistake * Update docs * Change glob patterns in `setup.cfg` (tardis-sn#2441) * Fix package_data in setup.cfg, mainly io and viz * more folders * Test pip installation * Remove test step * Fix more folders * restructure of geometry (tardis-sn#2422) * restructure of geometry * add radial1d boundary logic * black format * several fixes * fix epsilon * add testing of boundaries * change the r_inner_active * first integration with `from_config` working * hunting down density indexing bug * all model tests (without csvy) pass * more fixes * fix of model to simulation_state * fix inner boundary packet error * fix some leftovers * final fix for csvy * blackify * restructure to readers and remove some leftover code * further cleanup * first start of the restructure * add comment about removing quantitiness * add velocity check * add new abundance functions * remove default units * Add FAQ section to documentation (tardis-sn#2450) * Add faq * Added Overview * Fix typo * Remove lock file creation from pre release workflow (tardis-sn#2432) * Install latest version of mamba * Unpin conda lock installatiion * Split lockfilee creation to separate workflow * change docs installation- put a warning for conda forge installation and comment out install from package * fix * delete create lockfiles * download llock file * Pre-release 2023.10.20 (tardis-sn#2452) Automated changes for pre-release 2023.10.20 * Add verbose assert to NLTE rate equation (tardis-sn#2457) * add verbose assert * formatting * Fix bug in relativistic packet source (tardis-sn#2453) * Fix bug in relativistic packet source * Add initial test for BlackBodySimpleSourceRelativistic * Remove incorrect docstring * Update tardis/montecarlo/tests/test_packet_source.py Co-authored-by: Wolfgang Kerzendorf <[email protected]> * Update tardis/montecarlo/tests/test_packet_source.py Co-authored-by: Wolfgang Kerzendorf <[email protected]> * Rename all occurences of blackbodysimplesourcerelativistic * Do not hardcore blackbody_simplesource_relativistic.beta --------- Co-authored-by: Wolfgang Kerzendorf <[email protected]> * Enable nlte ionization as plasma component (tardis-sn#2458) * Rename nlte ion and electron * Enable NLTE * Fix typo * Add docs * Fix transitionprob dtype * dtype conversion * dtype for sparse matrix * Typing in numba * Fix test variable name * Add missing electron density case * Fix typo * Update docstr-cov.yml to fix failing action (tardis-sn#2462) * Update docstr-cov.yml to fix failing action * Explicitly install setuptools * NLTE Ionization solver polish (tardis-sn#2461) * assertion for first guess * Add checks to decay * Fix greater equal * Fix deprecated series index * Pre-release 2023.11.05 (tardis-sn#2466) Automated changes for pre-release 2023.11.05 * Tests Plasma Using Syrupy (tardis-sn#2413) * Add syrupy classes to conftest * Add syrupy to env file * Another folder for syrupy data * Save as npy instead of txt * Use syrupy fixtures * Different assertion functions for pandas dataframes and series * Zeta data fix * Blackify * Blackify conftest * test_hdf_plasmas using syrupy * Install syrupy with conda * Remove additional line adding syrupy as a plugin * Set update snapshots config option when generate reference is selected * Remove old refdata code from test plasmas complete * Create plasma conftest file * Text based snapshot fixture with custom location * Snapshot path * tardis regressions my fork * Refactor test_hdf_plasmas.py * Format using black * test nlte excitation using syrupy * nlte solver * plasma continuum * test tardis model density config * Lock files * Increase cache no * format using black * cleanup- sort dependencies, delete comments * Documentation to run these tests * remove commented out fixtures which are not used anywhere * docs code render fix * tests final cleanup * renaming files, yet to deal with ambr snapshot extension and curly brackets * renaming snapshots * Add comments * Add astropy import guard (tardis-sn#2470) * Add astropy import guard * mailmap * Reading in decay radiation data in atom data (tardis-sn#2471) * Reading in decay radiation data in atom data * Added docstring * Fixes the checksum * Pre-release 2023.11.26 (tardis-sn#2477) Automated changes for pre-release 2023.11.26 * Update plasma configuration documentation (tardis-sn#2459) * update plasma docs * Update mailmap * Fix typo * Add ruff rules and docs (tardis-sn#2478) * Add ruff rules and docs * Update pyproject.toml Co-authored-by: Wolfgang Kerzendorf <[email protected]> * rebuild docs --------- Co-authored-by: Wolfgang Kerzendorf <[email protected]> * Decay energy chain (tardis-sn#2448) * Added a function to calculate shell masses * Added a function to calculate shell masses * changed shell masses with the new function * Changed mass fraction to masses in to_inventories() * Co-authored-by: Wolfgang Kerzendorf <[email protected]> * Added a function to calculate total decays * Added a function to calculate energies from gamma rays and positrons. * added a function to calculate average energies of gamma rays and positrons * Added a fucntion to calculate each decay chain energies * Added dictionaries to handle multiple isotopes * Changed value to values * added tests for gamma_ray_transport * Added tests for calculating activity * Added test for activity * Added tests for two isotope * Changed Ni_isotope_mass * Added pytest paramterize * Added test for calculating shell masses * Ran test for checking activity of parent nuclide with analytical solution. * The function test_activity matches with the radioactivedecay output upto 14 decimal places. The Avogadro's number used in radioactivedecay is not from astropy. * Added tests for checking if iso_dict is returning the right key. * Added test for inventories dictionary. * Added a test to check if the calculate_average_energy function passes the right list ahead. * Added new function for testing energy budget from each decay chain. * Added a new function for energy per mass * Reading in decay radiation data in atom data * Add * Added tests for gamma ray transport. * Added tests for all functions for gamma_ray_transport. Added docstrings. * Changing decay energy chain * Added a function to get taus * Added tests for multiple isotopes * Fixes the test calculate shell masses with hand calculated values * Use Mamba instead of conda * Run asv for last 5 commits * Add pull request trigger temporarily * Reset --------- Co-authored-by: tardis-bot <[email protected]> Co-authored-by: Wolfgang Kerzendorf <[email protected]> Co-authored-by: Anirban <[email protected]> Co-authored-by: Alexander Holas <[email protected]> Co-authored-by: Alexander Holas <alexander.holas@h-its> Co-authored-by: Joshua Shields <[email protected]> Co-authored-by: Anirban <[email protected]> Co-authored-by: Christian Vogl <[email protected]> Co-authored-by: Andrew <[email protected]>

AlexHls requested review from wkerzendorf and sonachitchyan October 12, 2023 13:00

AlexHls added the bugfix 🪲 label Oct 12, 2023

wkerzendorf reviewed Oct 12, 2023

View reviewed changes

AlexHls commented Oct 12, 2023

View reviewed changes

AlexHls requested a review from chvogl October 12, 2023 13:08

AlexHls marked this pull request as ready for review October 12, 2023 18:28

AlexHls requested a review from wkerzendorf October 12, 2023 18:28

AlexHls mentioned this pull request Oct 12, 2023

Remove bandaid fix for duplicate linelists once upstream fix is available #2444

Open

wkerzendorf approved these changes Oct 12, 2023

View reviewed changes

wkerzendorf self-requested a review October 12, 2023 18:37

sonachitchyan previously approved these changes Oct 12, 2023

View reviewed changes

wkerzendorf requested changes Oct 12, 2023

View reviewed changes

AlexHls force-pushed the continuum-linelist branch from 69cde82 to 1e5d0f4 Compare October 12, 2023 18:45

AlexHls dismissed sonachitchyan’s stale review via 944a8a5 October 12, 2023 18:48

AlexHls force-pushed the continuum-linelist branch from 1e5d0f4 to 944a8a5 Compare October 12, 2023 18:48

AlexHls added 6 commits October 12, 2023 14:50

Workaround list access

df72e72

Formatting

a0f278b

Added error if duplicate

e77a44b

Remove duplicates

4dc6fe4

Fix typo, add comment

fcb2f74

Remove bandaid fix

df33990

AlexHls force-pushed the continuum-linelist branch from 944a8a5 to df33990 Compare October 12, 2023 18:50

Cleanup

0dc0b3d

More cleanup

d7ce1e7

AlexHls requested review from wkerzendorf and sonachitchyan October 12, 2023 18:55

sonachitchyan approved these changes Oct 13, 2023

View reviewed changes

wkerzendorf merged commit d2b0306 into tardis-sn:master Oct 13, 2023
8 checks passed

AlexHls deleted the continuum-linelist branch October 13, 2023 23:20

AlexHls mentioned this pull request Nov 30, 2023

Duplicate level index when using continuum interaction #2442

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix duplicate entries in continuum line list #2443

Fix duplicate entries in continuum line list #2443

AlexHls commented Oct 12, 2023

tardis-bot commented Oct 12, 2023

wkerzendorf Oct 12, 2023

AlexHls Oct 12, 2023

tardis-bot commented Oct 12, 2023

AlexHls Oct 12, 2023

tardis-bot commented Oct 12, 2023

tardis-bot commented Oct 12, 2023

wkerzendorf left a comment

codecov bot commented Oct 12, 2023 •

edited

Loading

review-notebook-app bot commented Oct 12, 2023

Fix duplicate entries in continuum line list #2443

Fix duplicate entries in continuum line list #2443

Conversation

AlexHls commented Oct 12, 2023

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

tardis-bot commented Oct 12, 2023

wkerzendorf Oct 12, 2023

Choose a reason for hiding this comment

AlexHls Oct 12, 2023

Choose a reason for hiding this comment

tardis-bot commented Oct 12, 2023

AlexHls Oct 12, 2023

Choose a reason for hiding this comment

tardis-bot commented Oct 12, 2023

tardis-bot commented Oct 12, 2023

wkerzendorf left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 12, 2023 • edited Loading

Codecov Report

review-notebook-app bot commented Oct 12, 2023

codecov bot commented Oct 12, 2023 •

edited

Loading