Skip to content

Conversation

@andrewfullard
Copy link
Contributor

@andrewfullard andrewfullard commented Dec 3, 2024

📝 Description

Type: 🚀 feature

Initial implementation of the new rates solver, level pop solver, and atomic data parsing into the plasma.

Relies on PRs tardis-sn/carsus#414 tardis-sn/carsus-regression-data#2 tardis-sn/tardis-regression-data#45

🚦 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.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tardis-bot
Copy link
Contributor

tardis-bot commented Dec 3, 2024

*beep* *bop*
Hi human,
I ran ruff on the latest commit (c9b63d4).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Details
2218	      	[ ] syntax-error
84	F405  	[ ] undefined-local-with-import-star-usage
33	I001  	[*] unsorted-imports
33	W293  	[ ] blank-line-with-whitespace
32	W291  	[ ] trailing-whitespace
16	F401  	[*] unused-import
10	S108  	[ ] hardcoded-temp-file
10	E701  	[ ] multiple-statements-on-one-line-colon
10	W605  	[*] invalid-escape-sequence
 9	E402  	[ ] module-import-not-at-top-of-file
 7	D202  	[*] blank-line-after-function
 4	ICN001	[ ] unconventional-import-alias
 4	G004  	[ ] logging-f-string
 4	W292  	[ ] missing-newline-at-end-of-file
 4	F821  	[ ] undefined-name
 3	E902  	[ ] io-error
 3	UP015 	[*] redundant-open-modes
 3	UP032 	[*] f-string
 2	C416  	[ ] unnecessary-comprehension
 2	RET505	[*] superfluous-else-return
 2	E702  	[ ] multiple-statements-on-one-line-semicolon
 2	E703  	[*] useless-semicolon
 1	EXE001	[ ] shebang-not-executable
 1	EXE003	[ ] shebang-missing-python
 1	G001  	[ ] logging-string-format
 1	RET506	[*] superfluous-else-raise
 1	F403  	[ ] undefined-local-with-import-star
 1	F541  	[*] f-string-missing-placeholders
 1	F811  	[*] redefined-while-unused
 1	RUF022	[*] unsorted-dunder-all
Found 2503 errors.
[*] 121 fixable with the `--fix` option (13 hidden fixes can be enabled with the `--unsafe-fixes` option).

Complete output(might be large):

Details
benchmarks/benchmark_base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
benchmarks/benchmark_base.py:74:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/io/atom_data/base.py:197:34: G004 Logging statement uses f-string
tardis/io/atom_data/base.py:259:17: G004 Logging statement uses f-string
tardis/io/atom_data/base.py:263:21: G001 Logging statement uses `str.format`
tardis/io/atom_data/base.py:707:17: G004 Logging statement uses f-string
tardis/plasma/properties/partition_function.py:21:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/partition_function.py:154:9: RET505 [*] Unnecessary `elif` after `return` statement
tardis/plasma/properties/partition_function.py:190:25: G004 Logging statement uses f-string
tardis/plasma/properties/property_collections.py:1:1: F403 `from tardis.plasma.properties import *` used; unable to detect undefined names
tardis/plasma/properties/property_collections.py:10:9: F405 `DilutePlanckianRadField` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:11:9: F405 `NumberDensity` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:12:9: F405 `TimeExplosion` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:13:9: F405 `AtomicData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:14:9: F405 `JBlues` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:15:9: F405 `LinkTRadTElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:16:9: F405 `HeliumTreatment` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:17:9: F405 `ContinuumInteractionSpecies` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:18:9: F405 `NLTEIonizationSpecies` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:19:9: F405 `NLTEExcitationSpecies` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:24:9: F405 `TRadiative` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:25:9: F405 `DilutionFactor` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:26:9: F405 `BetaRadiation` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:27:9: F405 `Levels` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:28:9: F405 `Lines` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:29:9: F405 `PartitionFunction` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:30:9: F405 `GElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:31:9: F405 `IonizationData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:32:9: F405 `LinesLowerLevelIndex` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:33:9: F405 `LinesUpperLevelIndex` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:34:9: F405 `StimulatedEmissionFactor` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:35:9: F405 `SelectedAtoms` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:36:9: F405 `ElectronTemperature` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:39:55: F405 `PhiSahaLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:40:55: F405 `LevelBoltzmannFactorLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:43:6: F405 `PhiSahaNebular` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:43:22: F405 `ZetaData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:43:32: F405 `BetaElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:43:46: F405 `RadiationFieldCorrection` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:46:6: F405 `LevelBoltzmannFactorDiluteLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:48:49: F405 `LevelBoltzmannFactorNoNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:51:9: F405 `LevelBoltzmannFactorNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:52:9: F405 `NLTEData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:53:9: F405 `PreviousElectronDensities` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:54:9: F405 `PreviousBetaSobolev` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:58:6: F405 `NLTEIndexHelper` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:58:23: F405 `NLTEPopulationSolverRoot` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:61:6: F405 `NLTEIndexHelper` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:61:23: F405 `NLTEPopulationSolverLU` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:65:9: F405 `HeliumNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:66:9: F405 `RadiationFieldCorrection` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:67:9: F405 `ZetaData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:68:9: F405 `BetaElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:69:9: F405 `LevelNumberDensityHeNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:70:9: F405 `IonNumberDensityHeNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:74:6: F405 `LevelNumberDensity` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:74:26: F405 `IonNumberDensity` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:77:6: F405 `HeliumNumericalNLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:81:9: F405 `PhotoIonRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:82:9: F405 `StimRecombRateFactor` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:83:9: F405 `BfHeatingRateCoeffEstimator` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:84:9: F405 `StimRecombCoolingRateCoeffEstimator` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:85:9: F405 `YgData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:90:9: F405 `StimRecombRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:91:9: F405 `PhotoIonizationData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:92:9: F405 `SpontRecombRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:93:9: F405 `ThermalLevelBoltzmannFactorLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:94:9: F405 `ThermalLTEPartitionFunction` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:95:9: F405 `BetaElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:96:9: F405 `ThermalGElectron` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:97:9: F405 `ThermalPhiSahaLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:98:9: F405 `SahaFactor` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:99:9: F405 `CorrPhotoIonRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:100:9: F405 `SpontRecombCoolingRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:101:9: F405 `YgInterpolator` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:102:9: F405 `CollExcRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:103:9: F405 `CollDeexcRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:104:9: F405 `RawCollisionTransProbs` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:105:9: F405 `MarkovChainIndex` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:106:9: F405 `FreeFreeCoolingRate` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:107:9: F405 `FreeBoundCoolingRate` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:108:9: F405 `LevelNumberDensityLTE` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:109:9: F405 `PhotoIonBoltzmannFactor` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:110:9: F405 `FreeBoundEmissionCDF` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:111:9: F405 `LevelIdxs2LineIdx` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:112:9: F405 `LevelIdxs2TransitionIdx` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:113:9: F405 `CollIonRateCoeffSeaton` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:114:9: F405 `CollRecombRateCoeff` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:115:9: F405 `ContinuumInteractionHandler` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:118:58: F405 `AdiabaticCoolingRate` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:121:9: F405 `RawTwoPhotonTransProbs` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:122:9: F405 `TwoPhotonData` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:123:9: F405 `TwoPhotonEmissionCDF` may be undefined, or defined from star imports
tardis/plasma/properties/property_collections.py:124:9: F405 `TwoPhotonFrequencySampler` may be undefined, or defined from star imports
Found 94 errors.
[*] 4 fixable with the `--fix` option.

@andrewfullard
Copy link
Contributor Author

This requires updated default atomic data to function correctly, using the full set of Chianti collisional strength values (and not the Carsus pre-calculated values).

@andrewfullard andrewfullard marked this pull request as ready for review January 15, 2025 22:04
@tardis-bot
Copy link
Contributor

tardis-bot commented Jan 15, 2025

*beep* *bop*

Hi, human.

The docs workflow has failed

Click here to see the build log.

@tardis-bot
Copy link
Contributor

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (cc0b125) and the latest commit (524968d).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

Details
| Change   | Before [5e2d0bb3] <master>   | After [524968d7]    |   Ratio | Benchmark (Parameter)                                                                             |
|----------|------------------------------|---------------------|---------|---------------------------------------------------------------------------------------------------|
| +        | 4.09±0.01ms                  | 6.27±0.01ms         |    1.53 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |

All benchmarks:

Details
Benchmarks that have stayed the same:

| Change   | Before [5e2d0bb3] <master>   | After [524968d7]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 1.31±0.3μs                   | 1.89±0.4μs          | ~1.44   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 591±200ns                    | 782±300ns           | ~1.32   | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 3.08±0.01ms                  | 3.56±0.03ms         | ~1.16   | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 2.02±1μs                     | 2.34±1μs            | ~1.16   | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 3.05±0.5μs                   | 3.52±0.2μs          | ~1.15   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 2.62±0.5ms                   | 2.86±0.4ms          | 1.09    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 43.8±20μs                    | 46.8±20μs           | 1.07    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 45.2±20μs                    | 46.3±20μs           | 1.02    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 63.4±0.1ms                   | 64.5±0.2ms          | 1.02    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 38.9±0.3s                    | 39.3±0.1s           | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 3.19±0.5μs                   | 3.23±0.7μs          | 1.01    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 592±200ns                    | 591±500ns           | 1.00    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 2.08±0m                      | 2.08±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 203±0.2ns                    | 203±0.03ns          | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 1.69±0.2ms                   | 1.69±0.01ms         | 1.00    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 6.32±0.8μs                   | 6.35±1μs            | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 1.05±0.01m                   | 1.04±0m             | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 735±0.8ns                    | 726±0.08ns          | 0.99    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 39.4±0.02μs                  | 38.5±0.02μs         | 0.98    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 601±100ns                    | 581±200ns           | 0.97    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 21.2±5μs                     | 20.6±5μs            | 0.97    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 1.27±0μs                     | 1.22±0μs            | 0.96    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 7.97±2μs                     | 7.38±2μs            | 0.93    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |

Benchmarks that have got worse:

| Change   | Before [5e2d0bb3] <master>   | After [524968d7]    |   Ratio | Benchmark (Parameter)                                                                             |
|----------|------------------------------|---------------------|---------|---------------------------------------------------------------------------------------------------|
| +        | 4.09±0.01ms                  | 6.27±0.01ms         |    1.53 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |

If you want to see the graph of the results, you can check it here

@andrewfullard
Copy link
Contributor Author

We need to change the new atomic data (version 2.0) to have .format_latest appended before the file type. This should be propagated to all sources of the data and all readers of the data. In the future an automated append of .format_latest would be nice. Then old versions would be renamed to .format_1.0 before the file type.

@andrewfullard
Copy link
Contributor Author

Double check where interpolation is happening

@andrewfullard
Copy link
Contributor Author

Double check where interpolation is happening

Interpolation is only happening for Chianti data in the UpsilonChiantiSolver class. The old NLTEData class remains for testing purposes.

@andrewfullard andrewfullard force-pushed the new-rates-implementation branch from 524968d to 1f8d852 Compare January 22, 2025 19:56
@andrewfullard andrewfullard marked this pull request as draft March 3, 2025 15:53
@andrewfullard andrewfullard force-pushed the new-rates-implementation branch from 1f8d852 to 7216bae Compare March 11, 2025 19:38
@andrewfullard
Copy link
Contributor Author

andrewfullard commented Mar 11, 2025

TODO:

  • Update regression data to use the raw Chianti values
  • Decide on a name for the new/old atomic data
  • Determine which data need updating
    • Standard data
    • Helium data
    • Others?
  • Update paths to new data
  • Try manual regression data comparison
  • Fix any broken tests

tardis-sn/tardis-regression-data#45

@andrewfullard andrewfullard mentioned this pull request May 6, 2025
6 tasks
@andrewfullard andrewfullard marked this pull request as ready for review May 7, 2025 17:23
@andrewfullard andrewfullard requested a review from jvshields May 29, 2025 15:07
@wkerzendorf wkerzendorf requested a review from Copilot May 29, 2025 15:24

# A fake electron distribution. Will eventually be a direct input
# to the plasma property.
electron_distribution = ThermalElectronEnergyDistribution(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still correct or was this a placeholder?

Or is this just needed to initialize the solver as a first guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a placeholder. Eventually t_electrons and previous_electron_densities should be in a ThermalElectronEnergyDistribution object and passed around together instead of as separate items. Ideally with a nonzero energy.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the references to the atomic data file throughout the documentation, configuration examples, notebooks, benchmarks, and CI workflows to use the new file "kurucz_cd23_chianti_H_He_latest.h5" for the refactored plasma rates solver implementation.

  • Updated file paths and download commands in various documentation and configuration files
  • Adjusted references in CI workflows and benchmarking scripts for consistent use of the new atomic data filename

Reviewed Changes

Copilot reviewed 86 out of 86 changed files in this pull request and generated no comments.

Show a summary per file
File Description
docs/io/configuration/components/models/densitycust/test_config.yml Updated atomic data filename
docs/io/configuration/components/models/data/config_no_init_trad.yml Updated atomic data filename
docs/io/configuration/components/models/data/config_init_trad.yml Updated atomic data filename
docs/io/configuration/components/models/Custom_TARDIS_Model_Tutorial.ipynb Updated atomic data filename in log messages
docs/io/configuration/components/atomic/current_public_table.rst Updated atomic data hyperlink and table entry
docs/contributing/tools/profiling/tardis_profiling_threads.ipynb Updated filename in sample code
docs/contributing/tools/profiling/profiling_example.ipynb Updated filename in sample code
docs/contributing/tools/benchmark.ipynb Updated filename in sample code
docs/contributing/in_progress/nebular_phase/gammaray_deposition.ipynb Updated atomic data configuration
docs/analyzing_tardis/visualization/tutorial_montecarlo_packet_visualization.ipynb Updated download command
docs/analyzing_tardis/visualization/tutorial_convergence_plot.ipynb Updated download command
docs/analyzing_tardis/visualization/how_to_sdec_plot.ipynb Updated download command
docs/analyzing_tardis/visualization/how_to_liv_plot.ipynb Updated download command
docs/analyzing_tardis/visualization/how_to_generating_widgets.ipynb Updated log messages and download command
docs/analyzing_tardis/liv_plot_notebook.ipynb Updated dataset recommendation and download command
benchmarks/data/tardis_configv1_benchmark.yml Updated atomic data filename
benchmarks/benchmark_base.py Updated atomic data import, check variable, and file path
.github/workflows/lfs-cache.yml Updated sparse checkout and LFS pull commands
.github/actions/setup_lfs/action.yml Updated sparse checkout and LFS checkout commands
.ci-helpers/download_reference_data.sh Updated atomic data filename in file list

@@ -0,0 +1,373 @@
{
Copy link
Contributor

@jvshields jvshields May 29, 2025

Choose a reason for hiding this comment

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

Might be a hassle, but is there anything that could be added to this plot to show that the NLTE species are doing something? Like also running a workflow without the NLTE balances and just showing that's it's different at all, or doing it from the number densities directly.


Reply via ReviewNB

nlte_data,
t_electrons,
j_blues,
dilute_planckian_radiation_field,
Copy link
Member

Choose a reason for hiding this comment

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

that should be any radiation field - or direct mean_intensities which @jvshields suggests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cannot be any radiation field in the current plasma, because this is a standard plasma input. The function that this is passed to takes a generic radiation field. The current plasma does not have a mean intensities property. Changes like that are outside the scope of this PR

@andrewfullard andrewfullard added the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@github-actions github-actions bot removed the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@andrewfullard
Copy link
Contributor Author

Sparse checkout of the cache (only pulling the atomic data) isn't working, because the workflows are not pulling the _latest atomic data. However, I have edited the workflows in this PR. This is expected behaviour- the workflows are using the master branch versions.

@andrewfullard andrewfullard added the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@github-actions github-actions bot removed the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@andrewfullard andrewfullard added the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@github-actions github-actions bot removed the git-lfs-pull Allow git lfs pull in PRs label May 30, 2025
@codecov
Copy link

codecov bot commented May 30, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.02%. Comparing base (d21716f) to head (c9b63d4).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
tardis/io/atom_data/nlte_data.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2896      +/-   ##
==========================================
- Coverage   69.63%   69.02%   -0.62%     
==========================================
  Files         232      232              
  Lines       16964    16917      -47     
==========================================
- Hits        11813    11677     -136     
- Misses       5151     5240      +89     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrewfullard andrewfullard merged commit 9260c57 into tardis-sn:master May 30, 2025
20 of 24 checks passed
atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Jul 2, 2025
* Implementation of the rates matrix and level pop solvers into plasma

* Demonstration notebook

* Fixes missing property for radiative NLTE and workflow notebook

* Fix legacy test

* Rename to use _latest atomic data file

* Resolve a couple of comments

* Correct Carsus read-in for new data files

* Locked in demo notebook

* Fix broken tests

- Corrected MD5 hash for the atomic data
- Renamed global because it is MD5 not UUID
- Removed legacy tests (not compatible with new data)

* Magic number change due to a change in atomic data

* Fix some more magic numbers

* Correct atomic data hash

* Fix hardcoded paths in notebooks

* Revert magic number change

Now matches the master branch and passes tests

* Revert another magic number change

* Revert magic number changes

* Fixes some docs pointing to the old data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants