V inner formal integral

Rodot- commented 4 weeks ago

:pencil:

Fixes the formal integral to handle changes in geometry

Type: :beetle: bugfix

When the formal integral is run with the v_inner solver, if the final active geometry state is different than that of the initial geometry mismatches in plasma and opacity quantities occur. This PR rectifies this by slicing these arrays to the correct size

Depends on PR #2797

:vertical_traffic_light: Testing

How did you test these changes?

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

CUDA version of the formal integral was tested locally

:ballot_box_with_check: Checklist

[x] 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[bot] commented 4 weeks ago

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

tardis-bot commented 4 weeks ago

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

```diff 12 G004 [ ] Logging statement uses f-string 8 I001 [*] Import block is un-sorted or un-formatted 8 D202 [*] No blank lines allowed after function docstring (found 1) 8 F401 [*] `astropy.units` imported but unused 4 RET505 [ ] Unnecessary `else` after `return` statement 4 W605 [*] Invalid escape sequence: `\A` 3 D209 [*] Multi-line docstring closing quotes should be on a separate line 3 UP008 [*] Use `super()` instead of `super(__class__, self)` 2 C403 [*] Unnecessary `list` comprehension (rewrite as a `set` comprehension) 2 RET506 [ ] Unnecessary `elif` after `raise` statement 2 F811 [ ] Redefinition of unused `OpacityState` from line 14 2 UP004 [*] Class `FormalIntegrator` inherits from `object` 1 C408 [*] Unnecessary `dict` call (rewrite as a literal) 1 G010 [*] Logging statement uses `warn` instead of `warning` 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 TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` 1 PTH117 [ ] `os.path.isabs()` should be replaced by `Path.is_absolute()` 1 E713 [*] Test for membership should be `not in` 1 E999 [ ] SyntaxError: Expected an expression 1 W291 [*] Trailing whitespace 1 W293 [*] Blank line contains whitespace ```

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:450: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:211:1: W293 [*] Blank line contains whitespace 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/formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/spectrum/formal_integral.py:9:25: F401 [*] `numba.char` imported but unused tardis/spectrum/formal_integral.py:9:31: F401 [*] `numba.float64` imported but unused tardis/spectrum/formal_integral.py:9:40: F401 [*] `numba.int64` imported but unused tardis/spectrum/formal_integral.py:9:47: F401 [*] `numba.typeof` imported but unused tardis/spectrum/formal_integral.py:9:55: F401 [*] `numba.byte` imported but unused tardis/spectrum/formal_integral.py:10:32: F401 [*] `numba.experimental.jitclass` imported but unused tardis/spectrum/formal_integral.py:21:5: F811 Redefinition of unused `opacity_state_initialize` from line 15 tardis/spectrum/formal_integral.py:22:5: F811 Redefinition of unused `OpacityState` from line 14 tardis/spectrum/formal_integral.py:22:5: F401 [*] `tardis.transport.montecarlo.numba_interface.OpacityState` imported but unused tardis/spectrum/formal_integral.py:55:5: D202 [*] No blank lines allowed after function docstring (found 1) tardis/spectrum/formal_integral.py:65:7: TD006 [*] Invalid TODO capitalization: `todo` should be `TODO` tardis/spectrum/formal_integral.py:218:29: UP004 [*] Class `NumbaFormalIntegrator` inherits from `object` tardis/spectrum/formal_integral.py:261:24: UP004 [*] Class `FormalIntegrator` inherits from `object` tardis/spectrum/formal_integral.py:299:9: D209 [*] Multi-line docstring closing quotes should be on a separate line tardis/spectrum/formal_integral.py:344:13: RET506 Unnecessary `else` after `raise` statement tardis/spectrum/formal_integral.py:356:16: E713 [*] Test for membership should be `not in` tardis/spectrum/formal_integral.py:409:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/spectrum/formal_integral.py:634:9: D209 [*] Multi-line docstring closing quotes should be on a separate line tardis/spectrum/formal_integral.py:703:5: RET505 Unnecessary `else` after `return` statement tardis/spectrum/formal_integral.py:739:5: RET505 Unnecessary `else` after `return` statement 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/visualization/tools/liv_plot.py:1:1: I001 [*] Import block is un-sorted or un-formatted tardis/visualization/tools/liv_plot.py:25:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/visualization/tools/liv_plot.py:48:9: D202 [*] No blank lines allowed after function docstring (found 1) tardis/visualization/tools/liv_plot.py:312:13: RET506 Unnecessary `elif` after `raise` statement tardis/visualization/tools/liv_plot.py:313:24: G010 [*] Logging statement uses `warn` instead of `warning` 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 69 errors. [*] 39 fixable with the `--fix` option (6 hidden fixes can be enabled with the `--unsafe-fixes` option). ```

tardis-bot commented 4 weeks ago

*beep* *bop*

Hi, human.

The docs workflow has failed :x:

Click here to see the build log.

codecov[bot] commented 4 weeks ago

Codecov Report

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

Project coverage is 67.61%. Comparing base (92df2cf) to head (5cb44b8). Report is 1 commits behind head on master.

Files	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 #2800 +/- ## ========================================== - Coverage 69.97% 67.61% -2.37% ========================================== Files 206 211 +5 Lines 15509 15940 +431 ========================================== - Hits 10853 10778 -75 - Misses 4656 5162 +506 ```

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

tardis-bot commented 4 weeks ago

*beep* *bop* Hi human, I ran benchmarks as you asked comparing master (714cffc8dd478b3db3bc6601bb205130b331b7f6) and the latest commit (5cb44b86c24065522c2e9e3d65f5e6ec3da07c4c). 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 [714cffc8] | After [5cb44b86] | Ratio | Benchmark (Parameter) | |----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------| | | 6.62±1μs | 7.51±1μs | ~1.13 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket | | | 49.6±30μs | 45.1±30μs | ~0.91 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter | | | 2.34±1μs | 2.01±2μs | ~0.86 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators | | | 3.23±0.05ms | 2.74±0ms | ~0.85 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') | | | 1.63±0.3μs | 1.36±0.4μs | ~0.83 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line | | | 5.33±0.4ms | 3.68±0.01ms | ~0.69 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') | | | 36.4±10μs | 24.2±6μs | ~0.66 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list | | | 205±0.2ns | 222±10ns | 1.08 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body | | | 3.20±0.4μs | 3.44±0.5μs | 1.08 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell | | | 571±200ns | 611±100ns | 1.07 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation | | | 47.6±20μs | 50.1±20μs | 1.05 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission | | | 63.3±0.06ms | 66.0±0.3ms | 1.04 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe | | | 2.57±0.4ms | 2.68±0.5ms | 1.04 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop | | | 1.20±0.01μs | 1.23±0μs | 1.02 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary | | | 39.5±0.02s | 39.7±0.01s | 1.01 | run_tardis.BenchmarkRunTardis.time_run_tardis | | | 30.7±0.03μs | 31.0±0.01μs | 1.01 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list | | | 532±200ns | 531±100ns | 1.00 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation | | | 2.07±0m | 2.07±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions | | | 1.06±0m | 1.05±0m | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking | | | 745±1ns | 737±1ns | 0.99 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter | | | 3.26±0.5μs | 3.23±0.6μs | 0.99 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket | | | 7.23±2μs | 7.08±2μs | 0.98 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley | | | 582±200ns | 561±100ns | 0.96 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation | | | 1.79±0.01ms | 1.72±0ms | 0.96 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop | ```

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

tardis-sn / tardis