Adds a radiation field convergence solver class #2791

andrewfullard · 2024-08-12T20:11:42Z

Has basically the same interface as ConvergenceSolver

📝 Description

Type: 🚀 feature

Radiative temperature and dilution factor have been unified in many parts of the code as a radiation field object. This PR creates a convergence class to handle them as one item, with the option to check convergence for them both simultaneously (currently defaults to off and is not settable).

🚦 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 · 2024-08-12T20:13:23Z

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

7	G004  	[ ] Logging statement uses f-string
2	F401  	[*] `tardis.io.configuration.config_reader.Configuration` imported but unused
1	RET505	[ ] Unnecessary `else` after `return` statement
1	RET506	[ ] Unnecessary `else` after `raise` statement
1	I001  	[*] Import block is un-sorted or un-formatted
1	UP008 	[*] Use `super()` instead of `super(__class__, self)`
1	TRY300	[ ] Consider moving this statement to an `else` block

Complete output(might be large):

tardis/simulation/base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/simulation/base.py:18:43: F401 [*] `tardis.plasma.radiation_field.DilutePlanckianRadiationField` imported but unused
tardis/simulation/base.py:158:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/simulation/base.py:199:13: RET506 Unnecessary `else` after `raise` statement
tardis/simulation/base.py:266:17: G004 Logging statement uses f-string
tardis/simulation/base.py:273:9: RET505 Unnecessary `else` after `return` statement
tardis/simulation/base.py:442:13: G004 Logging statement uses f-string
tardis/simulation/base.py:540:13: G004 Logging statement uses f-string
tardis/simulation/base.py:647:25: G004 Logging statement uses f-string
tardis/simulation/base.py:650:13: G004 Logging statement uses f-string
tardis/simulation/base.py:655:13: G004 Logging statement uses f-string
tardis/simulation/base.py:706:13: TRY300 Consider moving this statement to an `else` block
tardis/simulation/base.py:708:26: G004 Logging statement uses f-string
tardis/simulation/tests/test_simulation.py:7:51: F401 [*] `tardis.io.configuration.config_reader.Configuration` imported but unused
Found 14 errors.
[*] 3 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).

codecov · 2024-08-12T20:27:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.55%. Comparing base (4341331) to head (9b7263d).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2791      +/-   ##
==========================================
- Coverage   70.87%   70.55%   -0.33%     
==========================================
  Files         209      211       +2     
  Lines       15589    15619      +30     
==========================================
- Hits        11049    11020      -29     
- Misses       4540     4599      +59

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

tardis-bot · 2024-08-12T21:06:33Z

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

Significantly changed benchmarks:

All benchmarks:

Benchmarks that have stayed the same:

| Change   | Before [43413319] <master>   | After [9b7263df]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 2.70±0ms                     | 3.64±0.1ms          | ~1.35   | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 44.3±30μs                    | 59.2±30μs           | ~1.34   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 1.51±0.3μs                   | 1.37±0.4μs          | ~0.91   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 3.60±0.2μs                   | 3.09±0.4μs          | ~0.86   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 2.90±0.3μs                   | 3.17±0.4μs          | 1.10    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 1.91±1μs                     | 2.04±2μs            | 1.07    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 62.3±0.2ms                   | 65.9±0.01ms         | 1.06    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 1.18±0μs                     | 1.24±0μs            | 1.05    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 6.81±2μs                     | 7.12±1μs            | 1.05    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 592±100ns                    | 601±200ns           | 1.02    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 41.8±20μs                    | 42.5±20μs           | 1.02    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 3.62±0ms                     | 3.64±0.02ms         | 1.01    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 37.7±0.02s                   | 38.0±0.03s          | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.03±0m                      | 1.04±0m             | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 1.67±0ms                     | 1.68±0.01ms         | 1.01    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 551±200ns                    | 551±200ns           | 1.00    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 2.08±0m                      | 2.08±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 6.07±0.7μs                   | 6.04±0.9μs          | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 716±0.09ns                   | 707±0.5ns           | 0.99    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 561±100ns                    | 551±100ns           | 0.98    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 208±0.7ns                    | 203±0.08ns          | 0.98    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 23.7±6μs                     | 23.0±6μs            | 0.97    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 2.68±0.4ms                   | 2.54±0.5ms          | 0.95    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 34.3±0μs                     | 31.2±0.02μs         | 0.91    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |

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

tardis/simulation/radiation_field_convergence.py

tardis/simulation/base.py

DeerWhale · 2024-09-23T14:29:23Z

Need add tests, converting to draft for now.

Has basically the same interface as ConvergenceSolver

…e tests

jvshields · 2024-10-28T14:37:44Z

tardis/simulation/tests/test_radiation_field_convergence.py

+
+@pytest.fixture(scope="function")
+def partially_converged_radiation_field():
+    temperature = [1000, 2000, 3000] * u.K


Add a little documentation to explain what's going on here.

andrewfullard · 2024-10-28T14:37:54Z

tardis/simulation/tests/test_radiation_field_convergence.py

+
+@pytest.fixture(scope="function")
+def partially_converged_radiation_field():
+    temperature = [1000, 2000, 3000] * u.K


Add comment noting why this fixture is different

andrewfullard · 2024-10-28T14:39:31Z

tardis/simulation/tests/conftest.py

+
+
+@pytest.fixture(scope="function")
+def convergence_config(example_configuration_dir: Path):


Double check duplicate fixtures

jvshields · 2024-10-28T14:40:32Z

tardis/simulation/tests/conftest.py

+    )
+
+
+@pytest.fixture(scope="function")


Investigate why this is here and if any tests modify the fixture

andrewfullard added the restructure label Aug 12, 2024

andrewfullard requested review from wkerzendorf and Rodot- August 12, 2024 20:11

andrewfullard self-assigned this Aug 12, 2024

wkerzendorf reviewed Aug 13, 2024

View reviewed changes

tardis/simulation/radiation_field_convergence.py Show resolved Hide resolved

wkerzendorf reviewed Aug 13, 2024

View reviewed changes

tardis/simulation/radiation_field_convergence.py Show resolved Hide resolved

Rodot- requested changes Aug 15, 2024

View reviewed changes

tardis/simulation/base.py Outdated Show resolved Hide resolved

andrewfullard force-pushed the radfield-convergence-solver branch from 8b3ad36 to 47dd257 Compare August 15, 2024 19:00

Rodot- previously approved these changes Aug 15, 2024

View reviewed changes

andrewfullard requested a review from Rodot- August 19, 2024 14:41

andrewfullard marked this pull request as draft August 19, 2024 14:41

andrewfullard dismissed Rodot-’s stale review via ab9b1c3 August 19, 2024 18:27

andrewfullard marked this pull request as ready for review September 11, 2024 19:16

andrewfullard requested a review from wkerzendorf September 11, 2024 19:16

DeerWhale marked this pull request as draft September 23, 2024 14:29

andrewfullard added 4 commits October 8, 2024 09:18

Adds a radiation field convergence solver class

84bc945

Has basically the same interface as ConvergenceSolver

Clean up

90870d9

Use the correct radiation_field_state attribute

a984851

Add configuration items to convergence classes

a61c895

andrewfullard force-pushed the radfield-convergence-solver branch from ab9b1c3 to a61c895 Compare October 8, 2024 13:18

Adds radiation field convergence tests and cleans up other convergenc…

f371f1d

…e tests

andrewfullard marked this pull request as ready for review October 8, 2024 13:55

andrewfullard added 2 commits October 8, 2024 09:56

Applies black

35a4f08

Fixes tests, again

9b7263d

jvshields reviewed Oct 28, 2024

View reviewed changes

andrewfullard commented Oct 28, 2024

View reviewed changes

jvshields reviewed Oct 28, 2024

View reviewed changes

andrewfullard marked this pull request as draft November 4, 2024 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a radiation field convergence solver class #2791

Adds a radiation field convergence solver class #2791

andrewfullard commented Aug 12, 2024 •

edited

Loading

tardis-bot commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

tardis-bot commented Aug 12, 2024 •

edited

Loading

DeerWhale commented Sep 23, 2024

jvshields Oct 28, 2024

andrewfullard Oct 28, 2024

andrewfullard Oct 28, 2024

jvshields Oct 28, 2024



		@pytest.fixture(scope="function")
		def convergence_config(example_configuration_dir: Path):

Adds a radiation field convergence solver class #2791

Are you sure you want to change the base?

Adds a radiation field convergence solver class #2791

Conversation

andrewfullard commented Aug 12, 2024 • edited Loading

📝 Description

🚦 Testing

☑️ Checklist

tardis-bot commented Aug 12, 2024 • edited Loading

codecov bot commented Aug 12, 2024 • edited Loading

Codecov Report

tardis-bot commented Aug 12, 2024 • edited Loading

DeerWhale commented Sep 23, 2024

jvshields Oct 28, 2024

Choose a reason for hiding this comment

andrewfullard Oct 28, 2024

Choose a reason for hiding this comment

andrewfullard Oct 28, 2024

Choose a reason for hiding this comment

jvshields Oct 28, 2024

Choose a reason for hiding this comment

andrewfullard commented Aug 12, 2024 •

edited

Loading

tardis-bot commented Aug 12, 2024 •

edited

Loading

codecov bot commented Aug 12, 2024 •

edited

Loading

tardis-bot commented Aug 12, 2024 •

edited

Loading