Skip to content

Conversation

@erinvisser
Copy link
Contributor

@erinvisser erinvisser commented Apr 14, 2025

📝 Description

Type: 🚦 testing

Added tests to cover the Simple and Standard TARDIS Workflows using the config_verysimple and simulation_verysimple as defined in conftest.py.

Added tests to cover the v_inner_solver workflow. Created regression data needed for the test in this PR on the tardis-regression-data repo. .

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

@tardis-bot
Copy link
Contributor

tardis-bot commented Apr 14, 2025

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

Details
6631	       	[ ] syntax-error
35	E902   	[ ] io-error
26	W293   	[ ] blank-line-with-whitespace
26	I001   	[*] unsorted-imports
25	F821   	[ ] undefined-name
24	F401   	[ ] unused-import
24	G004   	[ ] logging-f-string
14	E701   	[ ] multiple-statements-on-one-line-colon
14	RET505 	[*] superfluous-else-return
13	F405   	[ ] undefined-local-with-import-star-usage
12	W605   	[*] invalid-escape-sequence
11	W291   	[ ] trailing-whitespace
 8	D202   	[*] blank-line-after-function
 8	UP008  	[ ] super-call-with-parameters
 6	E402   	[ ] module-import-not-at-top-of-file
 6	UP031  	[ ] printf-string-formatting
 4	E702   	[ ] multiple-statements-on-one-line-semicolon
 4	B011   	[ ] assert-false
 4	PT015  	[ ] pytest-assert-always-false
 4	UP015  	[*] redundant-open-modes
 3	W292   	[ ] missing-newline-at-end-of-file
 3	C408   	[ ] unnecessary-collection-call
 3	D209   	[*] new-line-after-last-paragraph
 3	NPY201 	[ ] numpy2-deprecation
 3	PTH117 	[ ] os-path-isabs
 2	E712   	[ ] true-false-comparison
 2	D411   	[*] no-blank-line-before-section
 2	F403   	[ ] undefined-local-with-import-star
 2	F811   	[ ] redefined-while-unused
 2	INP001 	[ ] implicit-namespace-package
 2	PGH004 	[ ] blanket-noqa
 2	RET506 	[*] superfluous-else-raise
 1	E722   	[ ] bare-except
 1	B017   	[ ] assert-raises-exception
 1	B019   	[ ] cached-instance-method
 1	B020   	[ ] loop-variable-overrides-iterator
 1	C416   	[ ] unnecessary-comprehension
 1	D406   	[*] missing-new-line-after-section-name
 1	D407   	[*] missing-dashed-underline-after-section
 1	ICN001 	[ ] unconventional-import-alias
 1	LOG015 	[ ] root-logger-call
 1	N812   	[ ] lowercase-imported-as-non-lowercase
 1	N999   	[ ] invalid-module-name
 1	PERF102	[ ] incorrect-dict-iterator
 1	PERF403	[ ] manual-dict-comprehension
 1	PIE790 	[*] unnecessary-placeholder
 1	PT021  	[ ] pytest-fixture-finalizer-callback
 1	RUF021 	[*] parenthesize-chained-operators
 1	S604   	[ ] call-with-shell-equals-true
 1	TRY300 	[ ] try-consider-else
 1	UP004  	[*] useless-object-inheritance
 1	UP009  	[*] utf8-encoding-declaration
 1	UP024  	[*] os-error-alias
 1	UP030  	[ ] format-literals
 1	UP032  	[*] f-string
Found 6951 errors.
[*] 113 fixable with the `--fix` option (37 hidden fixes can be enabled with the `--unsafe-fixes` option).

Complete output(might be large):

Details
benchmarks/spectrum_formal_integral.py:22:5: B019 Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks
docs/conf.py:1:1: UP009 [*] UTF-8 encoding declaration is unnecessary
docs/conf.py:27:1: I001 [*] Import block is un-sorted or un-formatted
docs/conf.py:30:8: F401 [*] `tardis` imported but unused
docs/conf.py:37:43: PGH004 Use specific rule codes when using `noqa`
docs/conf.py:47:27: UP015 [*] Unnecessary mode argument
docs/conf.py:68:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:69:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:70:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:71:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:72:1: F405 `exclude_patterns` may be undefined, or defined from star imports
docs/conf.py:142:1: W293 Blank line contains whitespace
docs/conf.py:177:1: W293 Blank line contains whitespace
docs/conf.py:194:13: UP030 Use implicit references for positional format fields
docs/conf.py:194:13: UP032 [*] Use f-string instead of `format` call
docs/conf.py:221:1: E402 Module level import not at top of file
docs/conf.py:294:4: E712 Avoid equality comparisons to `True`; use `toml_config_tool_dict["tardis"]['edit_on_github']:` for truth checks
docs/conf.py:316:1: E402 Module level import not at top of file
docs/conf.py:368:1: E402 Module level import not at top of file
docs/conf.py:383:37: UP015 [*] Unnecessary modes, use `w`
docs/conf.py:398:41: UP015 [*] Unnecessary modes, use `w`
docs/conf.py:408:55: W291 [*] Trailing whitespace
docs/conf.py:413:37: UP015 [*] Unnecessary modes, use `w`
docs/physics/pyplot/lte_ionization_balance.py:1:1: I001 [*] Import block is un-sorted or un-formatted
docs/physics/pyplot/lte_ionization_balance.py:1:8: F401 [*] `os` imported but unused
docs/physics/pyplot/lte_ionization_balance.py:22:31: F821 Undefined name `atom_fname`
docs/physics/pyplot/lte_ionization_balance.py:73:9: UP031 Use format specifiers instead of percent format
docs/physics/pyplot/lte_ionization_balance.py:74:15: UP031 Use format specifiers instead of percent format
docs/physics/pyplot/nebular_ionization_balance.py:1:1: I001 [*] Import block is un-sorted or un-formatted
docs/physics/pyplot/nebular_ionization_balance.py:33:18: F821 Undefined name `plasma`
docs/physics/pyplot/nebular_ionization_balance.py:71:9: UP031 Use format specifiers instead of percent format
docs/physics/pyplot/nebular_ionization_balance.py:72:15: UP031 Use format specifiers instead of percent format
docs/physics/pyplot/nebular_ionization_balance.py:117:9: UP031 Use format specifiers instead of percent format
docs/physics/pyplot/nebular_ionization_balance.py:118:15: UP031 Use format specifiers instead of percent format
tardis/conftest.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/conftest.py:12:1: F403 `from tardis.tests.fixtures.atom_data import *` used; unable to detect undefined names
tardis/conftest.py:15:36: F401 [*] `tardis.tests.test_util.monkeysession` imported but unused
tardis/conftest.py:18:12: F401 `tardisbase` imported but unused; consider using `importlib.util.find_spec` to test for availability
tardis/conftest.py:39:49: PGH004 Use specific rule codes when using `noqa`
tardis/conftest.py:41:9: F821 Undefined name `pytest_report_header`
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/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/gui/widgets.py:6:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:8:5: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:16:1: I001 [*] Import block is un-sorted or un-formatted
tardis/gui/widgets.py:16:8: ICN001 `matplotlib` should be imported as `mpl`
tardis/gui/widgets.py:17:1: F403 `from matplotlib.figure import *` used; unable to detect undefined names
tardis/gui/widgets.py:24:32: F401 [*] `matplotlib.patches.Circle` imported but unused
tardis/gui/widgets.py:29:30: F401 [*] `tardis.util` imported but unused
tardis/gui/widgets.py:36:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:43:23: F405 `Figure` may be undefined, or defined from star imports
tardis/gui/widgets.py:54:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:55:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:58:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:164:27: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:167:26: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:168:23: C408 Unnecessary `dict()` call (rewrite as a literal)
tardis/gui/widgets.py:186:9: RET505 [*] Unnecessary `elif` after `return` statement
tardis/gui/widgets.py:198:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:226:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:229:22: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:229:61: F405 `yaml` may be undefined, or defined from star imports
tardis/gui/widgets.py:353:24: F405 `TreeModel` may be undefined, or defined from star imports
tardis/gui/widgets.py:358:38: F405 `TreeDelegate` may be undefined, or defined from star imports
tardis/gui/widgets.py:429:35: UP024 [*] Replace aliased errors with `OSError`
tardis/gui/widgets.py:444:9: PIE790 [*] Unnecessary `pass` statement
tardis/gui/widgets.py:528:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/gui/widgets.py:646:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:657:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:660:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:688:41: F405 `np` may be undefined, or defined from star imports
tardis/gui/widgets.py:815:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:851:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:927:9: D209 [*] Multi-line docstring closing quotes should be on a separate line
tardis/gui/widgets.py:929:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1124:14: UP008 Use `super()` instead of `super(__class__, self)`
tardis/gui/widgets.py:1231:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/io/atom_data/util.py:35:13: G004 Logging statement uses f-string
tardis/io/configuration/config_internal.py:9:20: N812 Lowercase `__path__` imported as non-lowercase `TARDIS_PATH`
tardis/io/configuration/config_internal.py:25:13: G004 Logging statement uses f-string
tardis/io/configuration/config_internal.py:37:9: LOG015 `critical()` call on root logger
tardis/io/configuration/config_internal.py:38:13: G004 Logging statement uses f-string
tardis/io/configuration/config_reader.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/io/configuration/config_reader.py:54:29: G004 Logging statement uses f-string
tardis/io/configuration/config_reader.py:118:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/io/configuration/config_reader.py:142:13: RET505 [*] Unnecessary `else` after `return` statement
tardis/io/configuration/config_reader.py:219:29: G004 Logging statement uses f-string
tardis/io/configuration/tests/test_configuration_namespace.py:1:1: INP001 File `tardis/io/configuration/tests/test_configuration_namespace.py` is part of an implicit namespace package. Add an `__init__.py`.
tardis/io/model/parse_geometry_configuration.py:50:12: PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
tardis/io/model/parse_mass_fraction_configuration.py:4:25: F401 [*] `astropy.units` imported but unused
tardis/io/model/parse_mass_fraction_configuration.py:62:12: PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
tardis/io/model/readers/tests/test_csvy_reader.py:1:1: INP001 File `tardis/io/model/readers/tests/test_csvy_reader.py` is part of an implicit namespace package. Add an `__init__.py`.
tardis/io/model/readers/tests/test_csvy_reader.py:48:10: B017 Do not assert blind exception: `Exception`
tardis/io/util.py:20:20: F401 [*] `tardis.__version__` imported but unused
tardis/model/base.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/model/base.py:336:12: PTH117 `os.path.isabs()` should be replaced by `Path.is_absolute()`
tardis/model/base.py:371:21: 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/model/matter/decay.py:104:17: G004 Logging statement uses f-string
tardis/opacities/macro_atom/macroatom_solver.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/opacities/macro_atom/macroatom_solver.py:18:34: F401 [*] `astropy.constants` imported but unused
tardis/opacities/macro_atom/macroatom_solver.py:177:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/opacities/macro_atom/macroatom_solver.py:180:9: D411 [*] Missing blank line before section ("Parameters")
tardis/scripts/cmfgen2tardis.py:13:27: F821 Undefined name `atomic_dataset`
tardis/scripts/cmfgen2tardis.py:68:34: F821 Undefined name `atomic_dataset`
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/simulation/tests/test_simulation.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/simulation/tests/test_simulation.py:5:18: F401 [*] `copy.deepcopy` 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:14:5: F401 [*] `tardis.opacities.opacity_state.OpacityState` imported but unused
tardis/spectrum/formal_integral.py:358:13: RET506 [*] Unnecessary `else` after `raise` statement
tardis/spectrum/formal_integral.py:423:9: D202 [*] No blank lines allowed after function docstring (found 1)
tardis/spectrum/formal_integral.py:716:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral.py:752:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral.py:814: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/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/base.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/formal_integral.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/formal_integral_numba.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/source_function.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/tests/__init__.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/tests/test_formal_integral.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral/tests/test_source_function.py:1:1: E902 No such file or directory (os error 2)
tardis/spectrum/formal_integral_cuda.py:375:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/formal_integral_cuda.py:420:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/spectrum/tests/test_numba_formal_integral.py:35: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/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/transport/montecarlo/tests/test_montecarlo.py:27:1: E402 Module level import not at top of file
tardis/transport/montecarlo/tests/test_montecarlo.py:32:1: E402 Module level import not at top of file
tardis/transport/montecarlo/tests/test_montecarlo.py:48:5: PT021 Use `yield` instead of `request.addfinalizer`
tardis/transport/montecarlo/tests/test_montecarlo.py:204:5: F811 Redefinition of unused `test_get_random_mu_different_output` from line 195
tardis/transport/montecarlo/tests/test_montecarlo.py:484:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:570:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:571:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:579:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:596:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:601:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:624:9: F821 Undefined name `transport`
tardis/transport/montecarlo/tests/test_montecarlo.py:625:9: F821 Undefined name `transport`
tardis/transport/montecarlo/tests/test_montecarlo.py:626:9: F821 Undefined name `transport`
tardis/transport/montecarlo/tests/test_montecarlo.py:627:9: F821 Undefined name `transport`
tardis/transport/montecarlo/tests/test_montecarlo.py:629:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:642:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:666:5: F821 Undefined name `mc`
tardis/transport/montecarlo/tests/test_montecarlo.py:679:5: F821 Undefined name `mc`
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:341:5: RET505 [*] Unnecessary `else` after `return` statement
tardis/util/base.py:515:5: E722 Do not use bare `except`
tardis/util/base.py:623: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/custom_abundance.py:740:16: RUF021 [*] Parenthesize `a and b` expressions when chaining `and` and `or` together, to make the precedence clear
tardis/visualization/widgets/custom_abundance.py:1130:9: RET505 [*] Unnecessary `else` after `return` statement
tardis/visualization/widgets/custom_abundance.py:1421:9: RET506 [*] Unnecessary `else` after `raise` statement
tardis/visualization/widgets/custom_abundance.py:1724:9: F811 Redefinition of unused `input_d_time_0_eventhandler` from line 1713
tardis/visualization/widgets/custom_abundance.py:1754:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1756:13: F821 Undefined name `display`
tardis/visualization/widgets/custom_abundance.py:1758:13: F821 Undefined name `display`
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/standard_tardis_workflow.py:170:13: G004 Logging statement uses f-string
tardis/workflows/standard_tardis_workflow.py:210: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
tardis/workflows/tests/test_workflows.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/workflows/tests/test_workflows.py:2:21: F401 [*] `pathlib.Path` imported but unused
tardis/workflows/tests/test_workflows.py:7:51: F401 [*] `tardis.io.configuration.config_reader.Configuration` imported but unused
Found 235 errors.
[*] 87 fixable with the `--fix` option (37 hidden fixes can be enabled with the `--unsafe-fixes` option).

def test_simple_tardis_workflow(key, simulation_simple_workflow, simulation_verysimple):
actual = getattr(simulation_simple_workflow.simulation_state, key)
actual = actual.to_value() if hasattr(actual, "to_value") else actual
expected = getattr(simulation_verysimple.simulation_state, key)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think expected vs actual is a great way of phrasing these things. That makes more sense comparing something like a new value to regression data. Here I think you're comparing different workflows together, which is a useful test, but they're both freshly calculated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated the phrasing to be more informative.

from tardis.workflows.standard_tardis_workflow import StandardTARDISWorkflow

@pytest.fixture(scope="module")
def simulation_simple_workflow(config_verysimple, atomic_data_fname):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be called something like "workflow_simple" rather than "simulation_simple_workflow" They can have different attributes so keying in somebody else who might use this object that it is a different thing than a simulation object is important.

Similarly I'd probably rename sim to workflow as well.

These comments apply to all of these tests too, but should be an easy enough find and replace

Copy link
Contributor

Choose a reason for hiding this comment

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

Updated the naming to emphasize the workflow.

@andrewfullard
Copy link
Contributor

andrewfullard commented Apr 21, 2025

The tests seem to be failing to find the atomic data. Not sure if this is an issue with the workflows or the tests. I also wonder if we can just compare to the simulation regression data instead of the verysimple fixture (or if that is already happening anyway)

@andrewfullard andrewfullard self-assigned this Apr 28, 2025
@andrewfullard andrewfullard requested a review from jvshields April 28, 2025 14:17
@jvshields
Copy link
Contributor

Should investigate why the tests are failing because they can't find the atom data.

@tardis-bot
Copy link
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 .orcid.csv file in your current branch and push the changes to this pull request.

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:

email,orcid
[email protected],0000-0000-0000-0000

@DeerWhale DeerWhale requested a review from andrewfullard July 1, 2025 14:39
@DeerWhale
Copy link
Contributor

DeerWhale commented Jul 1, 2025

The tests seem to be failing to find the atomic data. Not sure if this is an issue with the workflows or the tests. I also wonder if we can just compare to the simulation regression data instead of the verysimple fixture (or if that is already happening anyway)

That's a good idea, now the standard_workflow is compared to the regression data produced for the test_simulation.py, the iteration values are consistent, but the estimators and output energies are somewhere different, looks like the run_tardis recompute the opacity state in the run_final while the workflow only recompute the MC state after convergence.

To match with the run_tardis, added the step recompute the opacity state in simple and standard workflow after convergence, now all test pass comparing the simulation regression data.

Since the simple workflow doesn't have iteration values logged by default, it is currently cross compared with the standard workflow results.

The v_inner test will fail for now till the newly generated regression data PR is merged.

@tardis-bot
Copy link
Contributor

tardis-bot commented Jul 1, 2025

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

Significantly changed benchmarks:

Details
| Change   | Before [7aee1a91] <master>   | After [c8ec5a42]    |   Ratio | Benchmark (Parameter)                                                                                                |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| +        | 1.14±0μs                     | 1.28±0μs            |    1.12 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |

All benchmarks:

Details
Benchmarks that have stayed the same:

| Change   | Before [7aee1a91] <master>   | After [c8ec5a42]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 45.8±30μs                    | 50.5±30μs           | ~1.10   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 3.30±0.3μs                   | 2.90±0.5μs          | ~0.88   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 571±100ns                    | 491±100ns           | ~0.86   | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 551±100ns                    | 471±200ns           | ~0.85   | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 541±200ns                    | 581±100ns           | 1.07    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 2.95±0.5ms                   | 3.14±0.4ms          | 1.06    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 3.34±0.02ms                  | 3.50±0.04ms         | 1.05    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 58.7±0.06ms                  | 61.3±0.03ms         | 1.05    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 2.11±0m                      | 2.13±0m             | 1.01    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 31.2±9μs                     | 31.4±9μs            | 1.01    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 3.43±0.5μs                   | 3.48±7μs            | 1.01    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 39.2±0.06s                   | 38.8±0.04s          | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.01±0m                      | 1.00±0m             | 0.99    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 199±0.1ns                    | 198±0.02ns          | 0.99    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 686±0.7ns                    | 681±2ns             | 0.99    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 7.03±3μs                     | 6.98±2μs            | 0.99    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 41.1±0.03μs                  | 39.1±0.2μs          | 0.95    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 6.68±1μs                     | 6.34±1μs            | 0.95    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 2.35±2μs                     | 2.22±1μs            | 0.94    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |
|          | 45.8±30μs                    | 43.3±30μs           | 0.94    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 4.46±0.01ms                  | 4.16±0ms            | 0.93    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 1.48±0.6μs                   | 1.36±0.4μs          | 0.92    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 2.81±0.01ms                  | 2.60±0.03ms         | 0.92    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |

Benchmarks that have got worse:

| Change   | Before [7aee1a91] <master>   | After [c8ec5a42]    |   Ratio | Benchmark (Parameter)                                                                                                |
|----------|------------------------------|---------------------|---------|----------------------------------------------------------------------------------------------------------------------|
| +        | 1.14±0μs                     | 1.28±0μs            |    1.12 | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary |

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

andrewfullard
andrewfullard previously approved these changes Jul 1, 2025
andrewfullard
andrewfullard previously approved these changes Jul 11, 2025
@DeerWhale
Copy link
Contributor

The test still failing because silly me generated the regression data in the wrong path, should be passing after merging this PR on regression-data repo: tardis-sn/tardis-regression-data#60

@andrewfullard
Copy link
Contributor

Weirdly Mac is failing, but Ubuntu passing

@andrewfullard
Copy link
Contributor

andrewfullard commented Jul 29, 2025

Likely an issue in a sort function somewhere. Need to use kind="stable" for ALL sorting algorithms.

@andrewfullard andrewfullard mentioned this pull request Aug 5, 2025
6 tasks
Copilot AI review requested due to automatic review settings August 6, 2025 18:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive test coverage for TARDIS workflows and improves code consistency. The changes primarily focus on testing the Simple and Standard TARDIS workflows against reference data, and introducing tests for the Inner Velocity Solver workflow with proper regression testing.

  • Adds test suite for Simple, Standard, and Inner Velocity Solver workflows
  • Refactors simulation tests to use shared configuration fixtures
  • Makes opacity_states an instance variable for better state management

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tardis/workflows/tests/test_workflows.py New comprehensive test suite for all three workflow types with regression testing
tardis/workflows/util.py Adds stable sorting to ensure deterministic behavior
tardis/workflows/standard_tardis_workflow.py Converts opacity_states to instance variable and minor formatting fixes
tardis/workflows/simple_tardis_workflow.py Converts opacity_states to instance variable for consistency
tardis/simulation/tests/test_simulation.py Refactors to use shared configuration fixture
tardis/conftest.py Adds reusable configuration fixture for simulation tests
.orcid.csv Adds ORCID identifiers for new contributors

Comment on lines +73 to +107
/ f"test_{attr_type}__{attr}__.h5"
)
ref_data = pd.read_hdf(ref_file)
if attr_type == "plasma_estimates":
if attr in ["nu_bar_estimator", "j_estimator"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.radfield_mc_estimators,
attr,
)
elif attr in ["output_nus", "output_energies"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.packet_collection,
attr,
)
else:
raise ValueError(f"Unknown plasma_estimates attr: {attr}")
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.Series(attr_data)
pd.testing.assert_series_equal(
attr_data, ref_data, check_exact=False, rtol=1e-6
)
elif attr_type == "plasma_state_iterations":
attr_data = getattr(standard_workflow_one_loop, attr)
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.DataFrame(attr_data)
pd.testing.assert_frame_equal(attr_data, ref_data, atol=0, rtol=1e-14)
else:
raise ValueError(f"Unknown attr_type: {attr_type}")


@pytest.mark.parametrize(
"attr",
[
Copy link

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The test function 'test_standard_tardis_workflow_against_run_tardis' has complex nested conditional logic that makes it difficult to maintain. Consider splitting this into separate test functions for 'plasma_estimates' and 'plasma_state_iterations' to improve readability and maintainability.

Suggested change
/ f"test_{attr_type}__{attr}__.h5"
)
ref_data = pd.read_hdf(ref_file)
if attr_type == "plasma_estimates":
if attr in ["nu_bar_estimator", "j_estimator"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.radfield_mc_estimators,
attr,
)
elif attr in ["output_nus", "output_energies"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.packet_collection,
attr,
)
else:
raise ValueError(f"Unknown plasma_estimates attr: {attr}")
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.Series(attr_data)
pd.testing.assert_series_equal(
attr_data, ref_data, check_exact=False, rtol=1e-6
)
elif attr_type == "plasma_state_iterations":
attr_data = getattr(standard_workflow_one_loop, attr)
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.DataFrame(attr_data)
pd.testing.assert_frame_equal(attr_data, ref_data, atol=0, rtol=1e-14)
else:
raise ValueError(f"Unknown attr_type: {attr_type}")
@pytest.mark.parametrize(
"attr",
[
/ f"test_plasma_estimates__{attr}__.h5"
)
ref_data = pd.read_hdf(ref_file)
if attr in ["nu_bar_estimator", "j_estimator"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.radfield_mc_estimators,
attr,
)
elif attr in ["output_nus", "output_energies"]:
attr_data = getattr(
standard_workflow_one_loop.transport_state.packet_collection,
attr,
)
else:
raise ValueError(f"Unknown plasma_estimates attr: {attr}")
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.Series(attr_data)
pd.testing.assert_series_equal(
attr_data, ref_data, check_exact=False, rtol=1e-6
)
@pytest.mark.parametrize(
"attr",
[
"iterations_w",
"iterations_t_rad",
"iterations_electron_densities",
"iterations_t_inner",
],
)
def test_standard_tardis_workflow_plasma_state_iterations(
standard_workflow_one_loop, attr, regression_data
):
ref_file = (
regression_data.regression_data_path
/ "tardis"
/ "simulation"
/ "tests"
/ "test_simulation"
/ f"test_plasma_state_iterations__{attr}__.h5"
)
ref_data = pd.read_hdf(ref_file)
attr_data = getattr(standard_workflow_one_loop, attr)
if hasattr(attr_data, "value"):
attr_data = attr_data.value
attr_data = pd.DataFrame(attr_data)
pd.testing.assert_frame_equal(attr_data, ref_data, atol=0, rtol=1e-14)
@pytest.mark.parametrize(
"attr",
[

Copilot uses AI. Check for mistakes.
@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
@andrewfullard andrewfullard merged commit 3ff94ab into tardis-sn:master Aug 6, 2025
26 of 33 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in DevOps Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants