Skip to content

Conversation

@hmperk18
Copy link
Contributor

@hmperk18 hmperk18 commented Jul 7, 2025

📝 Description

Type: 🎢 infrastructure

Reorganizing the formal integral make_source_function function into a solver class and adds testing to the computed values.

Also, repairs the cuda tests/benchmarks that were broken in PR #3181.

How did you test these changes?

  • Testing pipeline - on m2 macbook and moria
  • Other method (describe)

Requires tardis-regression-data PR#61.

@tardis-bot
Copy link
Contributor

tardis-bot commented Jul 7, 2025

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

@tardis-bot
Copy link
Contributor

tardis-bot commented Jul 7, 2025

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

Details
4948	       	[ ] syntax-error
25	E902   	[ ] io-error
23	I001   	[*] unsorted-imports
21	F401   	[*] unused-import
16	G004   	[ ] logging-f-string
12	W605   	[*] invalid-escape-sequence
11	D202   	[*] blank-line-after-function
10	W291   	[ ] trailing-whitespace
10	RET505 	[*] superfluous-else-return
 8	W293   	[*] blank-line-with-whitespace
 4	B011   	[ ] assert-false
 4	PT015  	[ ] pytest-assert-always-false
 3	F821   	[ ] undefined-name
 3	NPY201 	[ ] numpy2-deprecation
 2	D406   	[*] missing-new-line-after-section-name
 1	E402   	[ ] module-import-not-at-top-of-file
 1	E701   	[ ] multiple-statements-on-one-line-colon
 1	E712   	[ ] true-false-comparison
 1	E722   	[ ] bare-except
 1	W292   	[*] missing-newline-at-end-of-file
 1	B019   	[ ] cached-instance-method
 1	B020   	[ ] loop-variable-overrides-iterator
 1	C416   	[ ] unnecessary-comprehension
 1	D407   	[*] missing-dashed-underline-after-section
 1	D411   	[*] no-blank-line-before-section
 1	D412   	[*] blank-lines-between-header-and-content
 1	N999   	[ ] invalid-module-name
 1	PERF102	[ ] incorrect-dict-iterator
 1	PERF403	[ ] manual-dict-comprehension
 1	RET506 	[*] superfluous-else-raise
 1	S604   	[ ] call-with-shell-equals-true
 1	TRY300 	[ ] try-consider-else
 1	UP004  	[*] useless-object-inheritance
Found 5118 errors.
[*] 97 fixable with the `--fix` option (15 hidden fixes can be enabled with the `--unsafe-fixes` option).

Complete output(might be large):

Details
benchmarks/spectrum_formal_integral.py:5:1: I001 [*] Import block is un-sorted or un-formatted
benchmarks/spectrum_formal_integral.py:26:5: B019 Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks
tardis/energy_input/GXPacket.py:1:1: N999 Invalid module name: 'GXPacket'
tardis/energy_input/GXPacket.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/GXPacket.py:41:16: UP004 [*] Class `GXPacket` inherits from `object`
tardis/energy_input/GXPacket.py:75:9: D407 [*] Missing dashed underline after section ("Returns")
tardis/energy_input/GXPacket.py:75:9: D406 [*] Section name should end with a newline ("Returns")
tardis/energy_input/GXPacket.py:202:13: F821 Undefined name `get_chain_decay_power_per_ejectamass`
tardis/energy_input/GXPacket.py:203:17: F821 Undefined name `inventory`
tardis/energy_input/decay_radiation.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/energy_source.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/energy_source.py:96:11: E712 Avoid equality comparisons to `True`; use `check:` for truth checks
tardis/energy_input/energy_source.py:112:16: C416 Unnecessary list comprehension (rewrite using `list()`)
tardis/energy_input/energy_source.py:198:5: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/energy_input/gamma_ray_packet_source.py:1:25: F401 [*] `astropy.units` imported but unused
tardis/energy_input/gamma_ray_packet_source.py:294:53: F821 Undefined name `positronium_continuum`
tardis/energy_input/gamma_ray_transport.py:231:9: PERF403 Use a dictionary comprehension instead of a for-loop
tardis/energy_input/gamma_ray_transport.py:456:39: PERF102 When using only the values of a dict use the `values()` method
tardis/energy_input/main_gamma_ray_loop.py:166:17: G004 Logging statement uses f-string
tardis/energy_input/main_gamma_ray_loop.py:167:17: G004 Logging statement uses f-string
tardis/energy_input/main_gamma_ray_loop.py:211:17: G004 Logging statement uses f-string
tardis/energy_input/main_gamma_ray_loop.py:234:17: G004 Logging statement uses f-string
tardis/energy_input/main_gamma_ray_loop.py:235:17: G004 Logging statement uses f-string
tardis/energy_input/main_gamma_ray_loop.py:295:17: G004 Logging statement uses f-string
tardis/energy_input/tests/conftest.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/energy_input/tests/conftest.py:16:12: S604 Function call with truthy `shell` parameter identified, security issue
tardis/energy_input/tests/test_gamma_ray_channel.py:119:9: B020 Loop control variable `isotope_dict` overrides iterable it iterates
tardis/energy_input/tests/test_gamma_ray_grid.py:12:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_gamma_ray_grid.py:12:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_gamma_ray_grid.py:18:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_gamma_ray_grid.py:18:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_gamma_ray_interactions.py:16:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_gamma_ray_interactions.py:16:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_gamma_ray_interactions.py:22:5: PT015 Assertion always fails, replace with `pytest.fail()`
tardis/energy_input/tests/test_gamma_ray_interactions.py:22:12: B011 Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`
tardis/energy_input/tests/test_gamma_ray_packet_source_minimal.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/transport/GXPacket.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/transport/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/transport/tests/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/energy_input/util.py:409:5: RET505 [*] Unnecessary `elif` after `return` statement
tardis/energy_input/util.py:420:5: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/energy_input/util.py:422:5: D411 [*] Missing blank line before section ("Parameters")
tardis/io/model/parse_mass_fraction_configuration.py:4:25: F401 [*] `astropy.units` imported but unused
tardis/io/util.py:20:20: F401 [*] `tardis.__version__` imported but unused
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/model/matter/decay.py:104:17: G004 Logging statement uses f-string
tardis/simulation/base.py:245:17: G004 Logging statement uses f-string
tardis/simulation/base.py:418:13: G004 Logging statement uses f-string
tardis/simulation/base.py:519:13: G004 Logging statement uses f-string
tardis/simulation/base.py:599:13: G004 Logging statement uses f-string
tardis/simulation/base.py:604:13: G004 Logging statement uses f-string
tardis/simulation/base.py:655:13: TRY300 Consider moving this statement to an `else` block
tardis/simulation/base.py:657:26: G004 Logging statement uses f-string
tardis/spectrum/formal_integral/base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/base.py:3:24: F401 [*] `scipy.sparse` imported but unused
tardis/spectrum/formal_integral/base.py:4:31: F401 [*] `scipy.sparse.linalg` imported but unused
tardis/spectrum/formal_integral/base.py:8:33: F401 [*] `tardis.constants` imported but unused
tardis/spectrum/formal_integral/base.py:38:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/spectrum/formal_integral/base.py:115:5: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/formal_integral.py:5:31: F401 [*] `scipy.interpolate.interp1d` imported but unused
tardis/spectrum/formal_integral/formal_integral.py:81:9: D202 [*] No blank lines allowed after function docstring (found 1)
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_numba.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/formal_integral_numba.py:29:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_numba.py:59:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral/formal_integral_numba.py:134:12: 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/spectrum/formal_integral/source_function.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/source_function.py:16:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:28:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:151:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:232:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:280:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:310:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral/source_function.py:322:5: D412 [*] No blank lines allowed between a section header and its content ("Attributes")
tardis/spectrum/formal_integral/source_function.py:322:5: D406 [*] Section name should end with a newline ("Attributes")
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_formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/tests/test_formal_integral.py:7:55: F401 [*] `tardis.transport.montecarlo.configuration.montecarlo_globals` imported but unused
tardis/spectrum/formal_integral/tests/test_formal_integral.py:13:27: F401 [*] `tardis.spectrum.formal_integral.formal_integral_cuda.calculate_p_values` imported but unused
tardis/spectrum/formal_integral/tests/test_formal_integral.py:14:5: F401 [*] `tardis.spectrum.formal_integral.formal_integral_cuda.intensity_black_body_cuda` imported but unused
tardis/spectrum/formal_integral/tests/test_numba_formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/spectrum/formal_integral/tests/test_numba_formal_integral.py:8:61: F401 [*] `tardis.spectrum.formal_integral.formal_integral.FormalIntegrator` imported but unused
tardis/spectrum/formal_integral/tests/test_numba_formal_integral.py:47:16: 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/tests/test_tardis_full_formal_integral.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/tests/test_tardis_full_formal_integral.py:3:17: F401 [*] `numpy` imported but unused
tardis/transport/montecarlo/base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/transport/montecarlo/base.py:10:56: F401 [*] `tardis.opacities.continuum.continuum_state.ContinuumState` imported but unused
tardis/transport/montecarlo/base.py:12:5: F401 [*] `tardis.opacities.opacity_state.OpacityState` imported but unused
tardis/transport/montecarlo/packet_source.py:246:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/util/base.py:188:20: 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/util/base.py:339:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/util/base.py:513:5: E722 Do not use bare `except`
tardis/util/base.py:621:5: RET505 [*] Unnecessary `elif` after `return` statement
tardis/visualization/plot_util.py:54:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/tests/test_plot_util.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/visualization/widgets/tests/test_line_info.py:10:1: E402 Module level import not at top of file
tardis/workflows/high_energy/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/workflows/high_energy/tests/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/workflows/high_energy/tests/conftest.py:1:1: E902 No such file or directory (os error 2)
tardis/workflows/high_energy/tests/test_tardis_he_workflow.py:1:1: E902 No such file or directory (os error 2)
tardis/workflows/simple_tardis_workflow.py:249:17: G004 Logging statement uses f-string
tardis/workflows/simple_tardis_workflow.py:466:17: G004 Logging statement uses f-string
tardis/workflows/tardis_he_workflow.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/workflows/tardis_he_workflow.py:2:17: F401 [*] `numpy` imported but unused
tardis/workflows/tardis_he_workflow.py:4:25: F401 [*] `astropy.units` imported but unused
tardis/workflows/tardis_he_workflow.py:5:29: F401 [*] `astropy.constants` imported but unused
tardis/workflows/tardis_he_workflow.py:8:47: F401 [*] `tardis.energy_input.energy_source.get_nuclear_lines_database` imported but unused
tardis/workflows/tardis_he_workflow.py:72:1: W293 [*] Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:84:1: W293 Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:93:1: W293 [*] Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:103:1: W293 Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:107:90: W291 Trailing whitespace
tardis/workflows/tardis_he_workflow.py:112:1: W293 [*] Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:127:1: W293 Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:134:1: W293 [*] Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:139:1: W293 Blank line contains whitespace
tardis/workflows/tardis_he_workflow.py:169:44: W291 [*] Trailing whitespace
tardis/workflows/tardis_he_workflow.py:170:54: W291 [*] Trailing whitespace
tardis/workflows/tardis_he_workflow.py:193:34: W292 [*] No newline at end of file
Found 123 errors.
[*] 68 fixable with the `--fix` option (15 hidden fixes can be enabled with the `--unsafe-fixes` option).

@wkerzendorf
Copy link
Member

in this PR - can you make a sub package called formal integral.

@hmperk18
Copy link
Contributor Author

hmperk18 commented Jul 7, 2025

in this PR - can you make a sub package called formal integral.

Yep, I have the formal integral separated into a different package here in PR #3172. I can make this one rely on that. I was intending on blending them after #3172 was merged. I will double check everything there and make it not a draft.

@codecov
Copy link

codecov bot commented Jul 7, 2025

Codecov Report

❌ Patch coverage is 84.83412% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.72%. Comparing base (95358ad) to head (378f47f).
⚠️ Report is 17 commits behind head on master.

Files with missing lines Patch % Lines
...formal_integral/tests/test_cuda_formal_integral.py 10.00% 27 Missing ⚠️
...s/spectrum/formal_integral/formal_integral_cuda.py 50.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3177      +/-   ##
==========================================
+ Coverage   69.96%   71.72%   +1.75%     
==========================================
  Files         244      248       +4     
  Lines       17296    17359      +63     
==========================================
+ Hits        12102    12450     +348     
+ Misses       5194     4909     -285     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hmperk18 hmperk18 self-assigned this Jul 11, 2025
source_level_idx = ma_int_data.source_level_idx.values
destination_level_idx = ma_int_data.destination_level_idx.values

# if transport.line_interaction_type == "macroatom":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could make line_interaction_type a default kwarg passed to this function that the Solver class could then pass when performing calculations

self.line_interaction_type = line_interaction_type


def solve(self, sim_state, opacity_state, transport_state, atomic_data, levels):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The atomic_data should be part of the init, because it does not change between runs of the solver. levels is also part of the atomic data, so that doesn't need to be a separate argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I use atomic_data.levels instead of plasma.levels I get an error with the size of dataframes when computing e_dot_u. I moved the atomic_data but left the levels where they are for now

@Rodot- Rodot- self-requested a review August 5, 2025 19:35
@andrewfullard andrewfullard added the git-lfs-pull Allow git lfs pull in PRs label Aug 6, 2025
@github-actions github-actions bot removed the git-lfs-pull Allow git lfs pull in PRs label Aug 6, 2025
expected = regression_data.sync_ndarray(luminosity.cgs.value)
expected = u.Quantity(expected, "erg /s")
assert_quantity_allclose(luminosity, expected)
assert_quantity_allclose(luminosity, expected, rtol=1e-11, atol=0*u.erg/u.s)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Sorry if this has been asked before

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for all the tests we do - we always want to make sure to know exactly the tolerance.

Copy link
Member

@wkerzendorf wkerzendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

etst

Adjusting tolerances

Co-authored-by: Joshua Shields <[email protected]>
Co-authored-by: Jack O’Brien <[email protected]>
Co-authored-by: Jing Lu <[email protected]>

Co-authored-by: Wolfgang Kerzendorf <[email protected]>
Copy link
Contributor

@jvshields jvshields left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Atols I think we always want to be 0.

@andrewfullard andrewfullard merged commit 20e253f into tardis-sn:master Aug 6, 2025
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants