opacity_state_to_numba -> to_numba and moved to as a method of class OpacityState #2932

Sonu0305 · 2025-01-09T18:16:12Z

📝 Description

Type: 🎢 infrastructure

Change the one of the methods name:
opacity_state_to_numba -> to_numba
Moved to make it defined as one of the methods of class OpacityState

Closes #2882

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

This reverts commit 55a190d.

tardis-bot · 2025-01-09T18:18:14Z

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

3	F401  	[*] `tardis.opacities.continuum.continuum_state.ContinuumState` imported but unused
2	RET505	[ ] Unnecessary `else` after `return` statement
2	D202  	[*] No blank lines allowed after function docstring (found 1)
1	ANN204	[ ] Missing return type annotation for special method `__getitem__`
1	RET506	[ ] Unnecessary `else` after `raise` statement
1	I001  	[*] Import block is un-sorted or un-formatted
1	D406  	[*] Section name should end with a newline ("Returns")
1	D407  	[*] Missing dashed underline after section ("Returns")

Complete output(might be large):

tardis/opacities/opacity_state.py:110:9: ANN204 Missing return type annotation for special method `__getitem__`
tardis/opacities/opacity_state.py:116:9: D407 [*] Missing dashed underline after section ("Returns")
tardis/opacities/opacity_state.py:116:9: D406 [*] Section name should end with a newline ("Returns")
tardis/opacities/opacity_state.py:247:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/opacities/tests/test_opacity_state_numba.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/opacities/tests/test_opacity_state_numba.py:2:44: F401 [*] `tardis.opacities.opacity_state.OpacityState` imported but unused
tardis/spectrum/formal_integral.py:12:56: F401 [*] `tardis.opacities.continuum.continuum_state.ContinuumState` imported but unused
tardis/spectrum/formal_integral.py:362:13: RET506 Unnecessary `else` after `raise` statement
tardis/spectrum/formal_integral.py:427:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral.py:720:5: RET505 Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral.py:756:5: RET505 Unnecessary `else` after `return` statement
tardis/transport/montecarlo/base.py:10:56: F401 [*] `tardis.opacities.continuum.continuum_state.ContinuumState` imported but unused
Found 12 errors.
[*] 8 fixable with the `--fix` option.

…tyState" This reverts commit 65f1a66.

Sonu0305 · 2025-01-10T07:36:01Z

Hi @andrewfullard @atharva-2001 ,
Few tests are failing but I can't relate with the changes in this PR.
Please review this when you have a chance, Thank You.

atharva-2001 · 2025-01-10T08:08:48Z

Tests pass in master, that means it has something to with this PR right?
The Opacity tests fail-
tardis/opacities/tests/test_opacity_state_numba.py EEEEE

Sonu0305 · 2025-01-10T08:19:02Z

Tests pass in master, that means it has something to with this PR right? The Opacity tests fail- tardis/opacities/tests/test_opacity_state_numba.py EEEEE

Okay Thank You, I will look into it soon.

This reverts commit 55a190d.

…/tardis into fix#2882

codecov · 2025-01-14T13:42:58Z

Codecov Report

Attention: Patch coverage is 98.63014% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.42%. Comparing base (145a995) to head (2ecc751).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
tardis/spectrum/formal_integral.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2932      +/-   ##
==========================================
- Coverage   69.74%   69.42%   -0.32%     
==========================================
  Files         224      224              
  Lines       16313    16315       +2     
==========================================
- Hits        11377    11327      -50     
- Misses       4936     4988      +52

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

Sonu0305 · 2025-01-14T13:43:08Z

Hi @atharva-2001 @andrewfullard @wkerzendorf @KasukabeDefenceForce ,
The tests which previously failed are passing now, and also this PR is ready for review,
Please review this when you have a chance, Thank You.

tardis-bot · 2025-01-14T14:38:25Z

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (2a06fdf) and the latest commit (8c64aec).
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 [145a9956] <master>   | After [8c64aec3]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 40.7±30μs                    | 48.1±20μs           | ~1.18   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 2.97±0.4μs                   | 3.36±0.5μs          | ~1.13   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 6.14±0.7μs                   | 6.87±0.7μs          | ~1.12   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 652±100ns                    | 511±100ns           | ~0.78   | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 42.9±40μs                    | 45.5±30μs           | 1.06    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 2.86±0ms                     | 2.97±0.05ms         | 1.04    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 3.51±0.6μs                   | 3.66±0.3μs          | 1.04    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 22.9±6μs                     | 23.6±6μs            | 1.03    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 723±2ns                      | 737±0.3ns           | 1.02    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 7.05±2μs                     | 7.10±2μs            | 1.01    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 1.06±0m                      | 1.06±0m             | 1.00    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 2.08±0m                      | 2.08±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 39.8±0μs                     | 39.6±0.03μs         | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 67.4±0.1ms                   | 67.2±0.3ms          | 1.00    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 39.5±0.2s                    | 39.1±0.05s          | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.54±0.3μs                   | 1.51±0.3μs          | 0.98    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 214±0.09ns                   | 206±0.03ns          | 0.97    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 2.27±1μs                     | 2.17±1μs            | 0.96    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 1.79±0ms                     | 1.72±0.01ms         | 0.96    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 602±100ns                    | 571±100ns           | 0.95    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 2.83±0.4ms                   | 2.70±0.5ms          | 0.95    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 1.29±0μs                     | 1.22±0μs            | 0.94    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 601±100ns                    | 561±100ns           | 0.93    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 4.24±0.01ms                  | 3.92±0ms            | 0.92    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |

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

tardis/spectrum/formal_integral.py

tardis/transport/montecarlo/base.py

tardis/opacities/opacity_state.py

Sonu0305 · 2025-01-15T18:00:12Z

Hi @andrewfullard @jvshields ,
Could you please review the comments when you have a chance,
Also, Can the GSoC label be applied to this PR too?
Thank You for helping.

tardis/transport/montecarlo/base.py

Sonu0305 · 2025-01-22T16:51:33Z

Hi @andrewfullard ,
Can this be merged, so that I can start working on #2883, Thank You.

Sonu0305 · 2025-01-24T14:51:03Z

Thank You for your Time and Support,
@andrewfullard @jvshields

Sonu0305 added 5 commits January 2, 2025 23:26

for test

55a190d

Merge branch 'tardis-sn:master' into master

fee6b5f

Merge branch 'tardis-sn:master' into fix#2882

e750d09

Revert "for test"

db09932

This reverts commit 55a190d.

opacity_state_to_numba -> to_numba and also a method of OpacityState

65f1a66

Merge branch 'tardis-sn:master' into master

b6ce897

Sonu0305 marked this pull request as draft January 9, 2025 19:03

Sonu0305 added 2 commits January 10, 2025 00:33

Revert "opacity_state_to_numba -> to_numba and also a method of Opaci…

d0d10c1

…tyState" This reverts commit 65f1a66.

opacity_state_to_numba -> to_numba and also a method of OpacityState

0f50ffd

Sonu0305 marked this pull request as ready for review January 10, 2025 07:28

Sonu0305 marked this pull request as draft January 10, 2025 09:23

Sonu0305 added 9 commits January 12, 2025 01:52

Revert "for test"

6d3b92c

This reverts commit 55a190d.

Merge branch 'tardis-sn:master' into master

9f3097b

Merge branch 'tardis-sn:master' into master

f345db8

Merge branch 'tardis-sn:master' into master

0df035b

Merge branch 'tardis-sn:master' into master

e13e133

Merge branch 'tardis-sn:master' into fix#2882

322bbf0

Merge branches 'fix#2882' and 'master' of https://github.com/Sonu0305…

9461f98

…/tardis into fix#2882

maybe fixes tests failure

2b3dfd7

safe initialisation

5c5c5ef

Sonu0305 marked this pull request as ready for review January 14, 2025 13:41

jvshields reviewed Jan 14, 2025

View reviewed changes

tardis/spectrum/formal_integral.py Outdated Show resolved Hide resolved

jvshields reviewed Jan 14, 2025

View reviewed changes

tardis/transport/montecarlo/base.py Outdated Show resolved Hide resolved

jvshields reviewed Jan 14, 2025

View reviewed changes

tardis/opacities/opacity_state.py Show resolved Hide resolved

using from_plasma

8c64aec

andrewfullard reviewed Jan 21, 2025

View reviewed changes

tardis/transport/montecarlo/base.py Show resolved Hide resolved

Sonu0305 added 2 commits January 22, 2025 02:31

Merge branch 'master' into fix#2882

cdfbbda

using from_plasma

14221dd

andrewfullard requested review from jvshields and andrewfullard January 21, 2025 21:41

Missing from_plasma usage

2ecc751

andrewfullard approved these changes Jan 22, 2025

View reviewed changes

jvshields approved these changes Jan 22, 2025

View reviewed changes

andrewfullard merged commit e99368f into tardis-sn:master Jan 24, 2025
13 of 14 checks passed

Sonu0305 deleted the fix#2882 branch January 24, 2025 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

opacity_state_to_numba -> to_numba and moved to as a method of class OpacityState #2932

opacity_state_to_numba -> to_numba and moved to as a method of class OpacityState #2932

Sonu0305 commented Jan 9, 2025

tardis-bot commented Jan 9, 2025 •

edited

Loading

Sonu0305 commented Jan 10, 2025 •

edited

Loading

atharva-2001 commented Jan 10, 2025

Sonu0305 commented Jan 10, 2025

codecov bot commented Jan 14, 2025 •

edited

Loading

Sonu0305 commented Jan 14, 2025 •

edited

Loading

tardis-bot commented Jan 14, 2025 •

edited

Loading

Sonu0305 commented Jan 15, 2025

Sonu0305 commented Jan 22, 2025 •

edited

Loading

Sonu0305 commented Jan 24, 2025

opacity_state_to_numba -> to_numba and moved to as a method of class OpacityState #2932

opacity_state_to_numba -> to_numba and moved to as a method of class OpacityState #2932

Conversation

Sonu0305 commented Jan 9, 2025

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

tardis-bot commented Jan 9, 2025 • edited Loading

Sonu0305 commented Jan 10, 2025 • edited Loading

atharva-2001 commented Jan 10, 2025

Sonu0305 commented Jan 10, 2025

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

Sonu0305 commented Jan 14, 2025 • edited Loading

tardis-bot commented Jan 14, 2025 • edited Loading

Sonu0305 commented Jan 15, 2025

Sonu0305 commented Jan 22, 2025 • edited Loading

Sonu0305 commented Jan 24, 2025

tardis-bot commented Jan 9, 2025 •

edited

Loading

Sonu0305 commented Jan 10, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading

Sonu0305 commented Jan 14, 2025 •

edited

Loading

tardis-bot commented Jan 14, 2025 •

edited

Loading

Sonu0305 commented Jan 22, 2025 •

edited

Loading