search

tardis-sn / tardis

TARDIS - Temperature And Radiative Diffusion In Supernovae
https://tardis-sn.github.io/tardis
204 stars 406 forks source link

V inner boundary solver workflow #2797

Closed Rodot- closed 1 week ago

Rodot- commented 3 months ago

:pencil:

Adds a workflow to solve for the v_inner_boundary of the simulation

Based on PR #2730 Which should be merged first

Type: :rocket: feature

This adds a complete workflow as a subclass of the SimpleSimulation workflow from PR #2730. This workflow adds convergence criteria and handling the the inner-boundary velocity value. This PR also makes small edits to the configuration schema and parser to allow accessing properties of the v_inner_boundary convergence.

:pushpin: Resources

A notebook is provided in tardis/docs/workflows

:vertical_traffic_light: Testing

How did you test these changes?

:ballot_box_with_check: Checklist

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

review-notebook-app[bot] commented 3 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

tardis-bot commented 3 months ago

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

```diff 12 G004 [ ] Logging statement uses f-string 6 I001 [*] Import block is un-sorted or un-formatted 4 W605 [*] Invalid escape sequence: `\A` 4 D202 [*] No blank lines allowed after function docstring (found 1) 3 UP008 [*] Use `super()` instead of `super(__class__, self)` 2 C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension) 2 RET505 [ ] Unnecessary `else` after `return` statement 1 C408 [*] Unnecessary `dict` call (rewrite as a literal) 1 INP001 [ ] File `tardis/spectrum/tests/test_spectrum_solver.py` is part of an implicit namespace package. Add an `__init__.py`. 1 PIE790 [*] Unnecessary `pass` statement 1 TD005 [ ] Missing issue description after `TODO` 1 PTH117 [ ] `os.path.isabs()` should be replaced by `Path.is_absolute()` 1 E999 [ ] SyntaxError: Expected an expression 1 W291 [*] Trailing whitespace 1 D209 [*] Multi-line docstring closing quotes should be on a separate line 1 F401 [*] `astropy.units` imported but unused ```

Complete output(might be large):

```diff docs/workflows/simple_workflow.ipynb:cell 1:1:1: I001 [*] Import block is un-sorted or un-formatted docs/workflows/simple_workflow.ipynb:cell 7:10:26: W605 [*] Invalid escape sequence: `\A` docs/workflows/simple_workflow.ipynb:cell 7:11:40: W605 [*] Invalid escape sequence: `\A` docs/workflows/standard_workflow.ipynb:cell 1:1:1: I001 [*] Import block is un-sorted or un-formatted docs/workflows/standard_workflow.ipynb:cell 7:10:26: W605 [*] Invalid escape sequence: `\A` docs/workflows/standard_workflow.ipynb:cell 7:11:40: W605 [*] Invalid escape sequence: `\A` docs/workflows/v_inner_solver_workflow.ipynb:cell 1:1:1: I001 [*] Import block is un-sorted or un-formatted docs/workflows/v_inner_solver_workflow.ipynb:cell 3:14:23: W291 [*] Trailing whitespace tardis/io/configuration/config_reader.py:53:29: G004 Logging statement uses f-string tardis/io/configuration/config_reader.py:88:21: C408 Unnecessary `dict` call (rewrite as a literal) tardis/io/configuration/config_reader.py:112:21: UP008 Use `super()` instead of `super(__class__, self)` tardis/io/configuration/config_reader.py:117:9: RET505 Unnecessary `else` after `return` statement tardis/io/configuration/config_reader.py:118:18: UP008 Use `super()` instead of `super(__class__, self)` tardis/io/configuration/config_reader.py:141:13: RET505 Unnecessary `else` after `return` statement tardis/io/configuration/config_reader.py:218:29: G004 Logging statement uses f-string tardis/io/configuration/config_reader.py:455:14: UP008 Use `super()` instead of `super(__class__, self)` tardis/io/configuration/schemas/montecarlo_definitions.yml:1:13: E999 SyntaxError: Expected an expression tardis/model/base.py:344:12: PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()` tardis/model/base.py:357:27: C403 Unnecessary `list` comprehension (rewrite as a `set` comprehension) tardis/model/base.py:370:27: C403 Unnecessary `list` comprehension (rewrite as a `set` comprehension) tardis/model/base.py:381:21: G004 Logging statement uses f-string tardis/spectrum/tests/test_spectrum_solver.py:1:1: INP001 File `tardis/spectrum/tests/test_spectrum_solver.py` is part of an implicit namespace package. Add an `__init__.py`. tardis/workflows/simple_simulation.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/workflows/simple_simulation.py:160:21: G004 Logging statement uses f-string tardis/workflows/simple_simulation.py:263:17: G004 Logging statement uses f-string tardis/workflows/simple_simulation.py:402:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/workflows/simple_simulation.py:481:17: G004 Logging statement uses f-string tardis/workflows/standard_simulation.py:140:13: G004 Logging statement uses f-string tardis/workflows/standard_simulation.py:190:17: G004 Logging statement uses f-string tardis/workflows/util.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/workflows/util.py:1:30: F401 [*] `astropy.units` imported but unused tardis/workflows/util.py:6:5: D202 [*] No blank lines allowed after function docstring (found 1) tardis/workflows/v_inner_solver.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/workflows/v_inner_solver.py:16:3: TD005 Missing issue description after `TODO` tardis/workflows/v_inner_solver.py:47:9: D209 [*] Multi-line docstring closing quotes should be on a separate line tardis/workflows/v_inner_solver.py:52:9: PIE790 [*] Unnecessary `pass` statement tardis/workflows/v_inner_solver.py:93:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/workflows/v_inner_solver.py:119:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/workflows/v_inner_solver.py:180:17: G004 Logging statement uses f-string tardis/workflows/v_inner_solver.py:208:17: G004 Logging statement uses f-string tardis/workflows/workflow_logging.py:87:25: G004 Logging statement uses f-string tardis/workflows/workflow_logging.py:90:13: G004 Logging statement uses f-string Found 42 errors. [*] 18 fixable with the `--fix` option (6 hidden fixes can be enabled with the `--unsafe-fixes` option). ```
tardis-bot commented 3 months ago

*beep* *bop*

Hi, human.

The docs workflow has succeeded :heavy_check_mark:

Click here to see your results.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 1.68675% with 408 lines in your changes missing coverage. Please review.

Project coverage is 67.60%. Comparing base (8a317dd) to head (a301095). Report is 61 commits behind head on master.

Files with missing lines Patch % Lines
tardis/workflows/simple_simulation.py 0.00% 157 Missing :warning:
tardis/workflows/v_inner_solver.py 0.00% 107 Missing :warning:
tardis/workflows/standard_simulation.py 0.00% 71 Missing :warning:
tardis/workflows/util.py 0.00% 41 Missing :warning:
tardis/workflows/workflow_logging.py 0.00% 31 Missing :warning:
tardis/model/base.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2797 +/- ## ========================================== - Coverage 69.91% 67.60% -2.32% ========================================== Files 206 211 +5 Lines 15523 15933 +410 ========================================== - Hits 10853 10771 -82 - Misses 4670 5162 +492 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tardis-bot commented 3 months ago

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

Significantly changed benchmarks:

```diff ```

All benchmarks:

```diff Benchmarks that have stayed the same: | Change | Before [8a317dd9] | After [a301095c] | Ratio | Benchmark (Parameter) | |----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------| | | 23.5±6μs | 29.1±9μs | ~1.24 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list | | | 2.72±0.01ms | 3.05±0.01ms | ~1.12 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') | | | 551±100ns | 481±100ns | ~0.87 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation | | | 682±200ns | 592±200ns | ~0.87 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation | | | 3.55±0.7μs | 3.07±0.6μs | ~0.86 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell | | | 3.11±0.6μs | 3.26±0.3μs | 1.05 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket | | | 7.43±2μs | 7.78±2μs | 1.05 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley | | | 43.5±30μs | 45.2±30μs | 1.04 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter | | | 3.67±0.03ms | 3.79±0.01ms | 1.03 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') | | | 2.67±0.4ms | 2.75±0.5ms | 1.03 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop | | | 1.04±0m | 1.06±0m | 1.02 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking | | | 42.4±20μs | 43.1±20μs | 1.02 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission | | | 202±0.2ns | 204±0.2ns | 1.01 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body | | | 29.9±0.04μs | 30.1±0.03μs | 1.01 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list | | | 39.6±0.03s | 39.6±0.05s | 1.00 | run_tardis.BenchmarkRunTardis.time_run_tardis | | | 2.07±0m | 2.07±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions | | | 1.19±0μs | 1.19±0.01μs | 1.00 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary | | | 1.65±0.3μs | 1.65±0.3μs | 1.00 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line | | | 738±0.6ns | 735±0.8ns | 1.00 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter | | | 66.0±0.04ms | 66.0±0.07ms | 1.00 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe | | | 1.70±0.01ms | 1.68±0ms | 0.99 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop | | | 2.25±2μs | 2.13±2μs | 0.95 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators | | | 601±200ns | 561±200ns | 0.93 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation | | | 7.27±0.9μs | 6.72±0.9μs | 0.92 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket | ```

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

andrewfullard commented 4 weeks ago

@Rodot- this needs to be rebased now

andrewfullard commented 1 week ago

Closed in favour of #2800