Test the Cuda Formal Integral #3250
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
*beep* *bop* Hi, human. I'm the @tardis-bot and I noticed that your email is not associated with an ORCID ID in our database. Please add your email and ORCID ID to the If you don't have an ORCID ID yet, you can create one for free at orcid.org. ORCID IDs help ensure you get proper credit for your scientific contributions. The format should be: |
Contributor
|
*beep* *bop* Details8042 [ ] syntax-error
47 W293 [ ] blank-line-with-whitespace
38 E701 [ ] multiple-statements-on-one-line-colon
21 W291 [ ] trailing-whitespace
11 RET505 [*] superfluous-else-return
9 I001 [*] unsorted-imports
8 E702 [ ] multiple-statements-on-one-line-semicolon
8 F405 [ ] undefined-local-with-import-star-usage
8 UP008 [ ] super-call-with-parameters
6 G004 [ ] logging-f-string
5 W292 [ ] missing-newline-at-end-of-file
4 E722 [ ] bare-except
4 D202 [*] blank-line-after-function
4 F401 [*] unused-import
3 E902 [ ] io-error
3 B011 [ ] assert-false
3 C408 [ ] unnecessary-collection-call
3 D209 [*] new-line-after-last-paragraph
3 F821 [ ] undefined-name
3 FA102 [ ] future-required-type-annotation
3 PT015 [ ] pytest-assert-always-false
3 RUF022 [*] unsorted-dunder-all
2 COM818 [ ] trailing-comma-on-bare-tuple
2 NPY201 [ ] numpy2-deprecation
1 D411 [*] no-blank-line-before-section
1 F403 [ ] undefined-local-with-import-star
1 F541 [*] f-string-missing-placeholders
1 F811 [ ] redefined-while-unused
1 FA100 [ ] future-rewritable-type-annotation
1 FURB187 [ ] list-reverse-copy
1 G001 [ ] logging-string-format
1 ICN001 [ ] unconventional-import-alias
1 PIE790 [*] unnecessary-placeholder
1 RET506 [*] superfluous-else-raise
1 RUF021 [*] parenthesize-chained-operators
1 UP024 [*] os-error-alias
Found 8255 errors.
[*] 52 fixable with the `--fix` option (21 hidden fixes can be enabled with the `--unsafe-fixes` option).
Complete output(might be large): Detailstardis/analysis.py:96:9: E722 Do not use bare `except`
tardis/analysis.py:110:9: E722 Do not use bare `except`
tardis/analysis.py:121:9: E722 Do not use bare `except`
tardis/analysis.py:136:9: E722 Do not use bare `except`
tardis/analysis.py:374:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/analysis.py:396:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/analysis.py:421:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/configuration/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/configuration/sorting_globals.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/main_gamma_ray_loop.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/main_gamma_ray_loop.py:30:23: FA100 Add `from __future__ import annotations` to simplify `typing.Optional`
tardis/energy_input/samplers.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/samplers.py:155:49: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.
tardis/energy_input/samplers.py:178:1: W293 [*] Blank line contains whitespace
tardis/energy_input/samplers.py:190:53: W291 [*] Trailing whitespace
tardis/energy_input/samplers.py:191:35: W291 [*] Trailing whitespace
tardis/energy_input/samplers.py:192:28: W292 [*] No newline at end of file
tardis/energy_input/tests/test_energy_source.py:35:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:35:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_energy_source.py:41:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:41:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_energy_source.py:47:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_energy_source.py:47:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/gui/widgets.py:7:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:9:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:17:1: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:17:8: ICN001 `matplotlib` should be imported as `mpl`
tardis/gui/widgets.py:18:1: F403 `from matplotlib.figure import *` used; unable to detect undefined names
tardis/gui/widgets.py:25:32: F401 [*] `matplotlib.patches.Circle` imported but unused
tardis/gui/widgets.py:30:30: F401 [*] `tardis.util` imported but unused
tardis/gui/widgets.py:37:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:44:23: F405 `Figure` may be undefined, or defined from star imports
tardis/gui/widgets.py:55:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:56:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:59:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:165:27: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:168:26: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:169:23: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:187:9: RET505 [*] Unnecessary `elif` after `return` statement
tardis/gui/widgets.py:199:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:227:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:230:22: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:230:61: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:354:24: F405 `TreeModel` may be undefined, or defined from star imports
tardis/gui/widgets.py:359:38: F405 `TreeDelegate` may be undefined, or defined from star imports
tardis/gui/widgets.py:430:35: UP024 [*] Replace aliased errors with `OSError`
tardis/gui/widgets.py:445:9: PIE790 [*] Unnecessary `pass` statement
tardis/gui/widgets.py:529:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:647:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:658:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:661:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:689:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:816:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:852:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:928:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:930:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1125:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1232:9: D202 [*] No blank lines allowed after function docstring (found 1)
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/model/matter/composition.py:173:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/model/matter/decay.py:101:21: G004 Logging statement uses f-string
tardis/opacities/macro_atom/macroatom_state.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/opacities/macro_atom/macroatom_state.py:127:9: D411 [*] Missing blank line before section ("Returns")
tardis/opacities/macro_atom/macroatom_state.py:168:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/opacities/macro_atom/transition_probabilities.py:15:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/atomic.py:27:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/atomic.py:554:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/atomic.py:585:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/atomic.py:598:17: G004 Logging statement uses f-string
tardis/plasma/properties/atomic.py:639:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/plasma/properties/continuum_processes/rates.py:22:11: RUF022 [*] `__all__` is not sorted
tardis/plasma/properties/continuum_processes/rates.py:156:13: FURB187 Use of assignment of `reversed` on list `idx_arrays`
tardis/spectrum/formal_integral/formal_integral_cuda.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/formal_integral_cuda.py:85:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_cuda.py:124:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_solver.py:38:13: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
tardis/spectrum/formal_integral/formal_integral_solver.py:41:61: FA102 Missing `from __future__ import annotations`, but uses PEP 604 union
tardis/spectrum/formal_integral/formal_integral_solver.py:100:17: F541 [*] f-string without any placeholders
tardis/spectrum/formal_integral/formal_integral_solver.py:100:17: G004 Logging statement uses f-string
tardis/spectrum/formal_integral/formal_integral_solver.py:315:10: FA102 Missing `from __future__ import annotations`, but uses PEP 585 collection
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:9:5: F401 [*] `tardis.spectrum.formal_integral.base.interpolate_integrator_quantities` imported but unused
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py:15:61: F401 [*] `tardis.spectrum.formal_integral.source_function.SourceFunctionSolver` imported but unused
tardis/transport/montecarlo/estimators/util.py:66:32: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.
tardis/visualization/widgets/custom_abundance.py:737:16: RUF021 [*] Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
tardis/visualization/widgets/custom_abundance.py:1127:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/widgets/custom_abundance.py:1418:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/visualization/widgets/custom_abundance.py:1721:9: F811 Redefinition of unused `input_d_time_0_eventhandler` from line 1710
tardis/visualization/widgets/custom_abundance.py:1751:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1753:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1755:13: F821 Undefined name `display`
tardis/visualization/widgets/line_info.py:3:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/widgets/line_info.py:450:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:454:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:458:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:462:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:467:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:476:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:531:1: W293 [*] Blank line contains whitespace
tardis/visualization/widgets/line_info.py:715:1: W293 [*] Blank line contains whitespace
Found 103 errors.
[*] 52 fixable with the `--fix` option (21 hidden fixes can be enabled with the `--unsafe-fixes` option).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3250 +/- ##
==========================================
- Coverage 67.96% 67.79% -0.18%
==========================================
Files 172 173 +1
Lines 13211 13242 +31
==========================================
- Hits 8979 8977 -2
- Misses 4232 4265 +33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Actually using the |
Contributor
Author
|
however, issue #3251 may be related |
Contributor
|
*beep* *bop* Significantly changed benchmarks: Details| Change | Before [83384e88] <master> | After [2201a30e] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| - | 1.25±0.02μs | 1.14±0μs | 0.91 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
All benchmarks: DetailsBenchmarks that have improved:
| Change | Before [83384e88] <master> | After [2201a30e] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| - | 1.25±0.02μs | 1.14±0μs | 0.91 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |
Benchmarks that have stayed the same:
| Change | Before [83384e88] <master> | After [2201a30e] | Ratio | Benchmark (Parameter) |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
| | 25.5±7μs | 28.8±8μs | ~1.13 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
| | 4.27±0.02ms | 3.80±0ms | ~0.89 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom') |
| | 6.76±1μs | 5.88±1μs | ~0.87 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket |
| | 1.73±0.6μs | 1.43±0.5μs | ~0.83 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line |
| | 2.33±2μs | 1.95±1μs | ~0.83 | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators |
| | 621±200ns | 501±200ns | ~0.81 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation |
| | 43.1±20μs | 45.7±20μs | 1.06 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission |
| | 36.7±0.01s | 38.4±0.5s | 1.05 | run_tardis.BenchmarkRunTardis.time_run_tardis |
| | 1.10±0μs | 1.15±0μs | 1.04 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body |
| | 40.7±0.5μs | 41.2±0.04μs | 1.01 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list |
| | 2.77±0.3ms | 2.81±0.5ms | 1.01 | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop |
| | 1.05±0m | 1.05±0m | 1.00 | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions |
| | 58.5±0.03s | 57.8±0.1s | 0.99 | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking |
| | 501±100ns | 491±200ns | 0.98 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation |
| | 61.1±0.2ms | 60.1±0.1ms | 0.98 | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe |
| | 3.04±0.01ms | 2.94±0.02ms | 0.97 | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter') |
| | 3.38±0.5μs | 3.27±0.5μs | 0.97 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell |
| | 695±1ns | 665±0.3ns | 0.96 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter |
| | 2.65±0.01ms | 2.54±0ms | 0.96 | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop |
| | 7.36±3μs | 7.00±3μs | 0.95 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley |
| | 48.6±30μs | 45.6±30μs | 0.94 | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter |
| | 3.13±0.6μs | 2.88±0.6μs | 0.92 | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket |
| | 581±200ns | 531±200ns | 0.91 | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation |
If you want to see the graph of the results, you can check it here |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issues in the FormalIntegralSolver related to implementation selection and variable naming in CUDA tests. The changes ensure proper differentiation between CUDA and Numba implementations during testing.
Key changes:
- Simplified method selection logic in
FormalIntegralSolverto handle explicit method choices correctly - Fixed array indexing bug in CUDA formal integral implementation
- Enhanced test assertions to verify correct integrator types are being used
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tardis/spectrum/formal_integral/tests/test_cuda_formal_integral.py |
Removed redundant shell interpolation setup and added assertions to verify correct integrator types |
tardis/spectrum/formal_integral/formal_integral_solver.py |
Refactored method selection logic to properly handle explicit method specifications |
tardis/spectrum/formal_integral/formal_integral_cuda.py |
Fixed array indexing bug and corrected array dimension comments |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
andrewfullard
previously approved these changes
Sep 4, 2025
Rodot-
reviewed
Sep 5, 2025
andrewfullard
approved these changes
Sep 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Description
Type: 🪲
bugfixFix how the
FormalIntegralSolverhandles choosing an implementation and the variable names in the cudatest_full_formal_integralsuch that the cuda implementation is used to computeL_cudaand the numba implementation is used to computeL_numba.Additionally, update the cuda implementation to initialize and use the
zandshell_idarrays correctly