Disable virtual packets by default by wkerzendorf · Pull Request #3437 · tardis-sn/tardis

wkerzendorf · 2026-02-17T14:15:32Z

No description provided.

benchmarks/data/tardis_configv1_benchmark.yml

tardis/io/configuration/tests/data/tardis_configv1_verysimple.yml

andrewfullard · 2026-02-17T14:16:20Z

docs/tardis_example.yml


  last_no_of_packets: 1.e+5
-  no_of_virtual_packets: 10
+  # Virtual packets are computationally expensive and only needed for virtual spectrum generation


I don't think we should change tardis example

tardis/visualization/conftest.py

tardis-bot · 2026-02-17T14:18:29Z

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

Details

15372	      	[ ] syntax-error
 10	E701  	[ ] multiple-statements-on-one-line-colon
  9	W293  	[ ] blank-line-with-whitespace
  3	E702  	[ ] multiple-statements-on-one-line-semicolon
  2	D209  	[*] new-line-after-last-paragraph
  2	F401  	[ ] unused-import
  1	W291  	[ ] trailing-whitespace
  1	W292  	[ ] missing-newline-at-end-of-file
  1	D202  	[*] blank-line-after-function
  1	DTZ011	[ ] call-date-today
  1	F403  	[ ] undefined-local-with-import-star
  1	F821  	[ ] undefined-name
  1	I001  	[*] unsorted-imports
  1	INP001	[ ] implicit-namespace-package
  1	PGH004	[ ] blanket-noqa
  1	S113  	[ ] request-without-timeout
  1	UP034 	[ ] extraneous-parentheses
Found 15409 errors.
[*] 7 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).

Complete output(might be large):

Details

.ci-helpers/update_credits.py:1:1: INP001 File `.ci-helpers/update_credits.py` is part of an implicit namespace package. Add an `__init__.py`.
.ci-helpers/update_credits.py:1:1: D209 [*] Multi-line docstring closing quotes should be on a separate line
.ci-helpers/update_credits.py:15:5: D202 [*] No blank lines allowed after function docstring (found 1)
.ci-helpers/update_credits.py:15:5: D209 [*] Multi-line docstring closing quotes should be on a separate line
.ci-helpers/update_credits.py:20:12: DTZ011 `datetime.date.today()` used
.ci-helpers/update_credits.py:24:20: S113 Probable use of `requests` call without timeout
.ci-helpers/update_credits.py:46:91: W291 Trailing whitespace
tardis/conftest.py:1:1: I001 [*] Import block is un-sorted or un-formatted
tardis/conftest.py:14:1: F403 `from tardis.tests.fixtures.atom_data import *` used; unable to detect undefined names
tardis/conftest.py:19:36: F401 [*] `tardis.tests.test_util.monkeysession` imported but unused
tardis/conftest.py:22:12: F401 `tardisbase` imported but unused; consider using `importlib.util.find_spec` to test for availability
tardis/conftest.py:43:49: PGH004 Use specific rule codes when using `noqa`
tardis/conftest.py:45:9: F821 Undefined name `pytest_report_header`
tardis/conftest.py:339:1: W293 [*] Blank line contains whitespace
tardis/tests/test_tardis_full.py:68:1: W293 [*] Blank line contains whitespace
Found 15 errors.
[*] 7 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).

Copilot

Pull request overview

This PR disables virtual packets by default across the TARDIS codebase to improve performance. Virtual packets are computationally expensive and only needed for virtual spectrum generation, so they are now set to 0 by default in configuration files and test fixtures.

Changes:

Updated default configuration files to set no_of_virtual_packets=0 and virtual_packet_logging=False
Modified test fixtures to disable virtual packets by default
Explicitly enabled virtual packets in specific tests that require them (e.g., test_virtual_spectrum)

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tardis/visualization/conftest.py	Updated `simulation_simple_tracked` and `workflow_simple_tracked` fixtures to disable virtual packets by default
tardis/tests/test_tardis_full.py	Added explicit virtual packet enablement for `test_virtual_spectrum` test
tardis/io/configuration/tests/data/tardis_configv1_verysimple.yml	Set default `no_of_virtual_packets` to 0 and `virtual_packet_logging` to False
tardis/conftest.py	Updated `simulation_verysimple_vpacket_tracking` and `workflow_simple` fixtures to handle virtual packets appropriately
docs/tardis_example.yml	Updated example configuration to disable virtual packets by default
docs/io/optional/tardis_config_logger.yml	Updated example configuration to disable virtual packets by default
benchmarks/data/tardis_configv1_benchmark.yml	Disabled virtual packets in benchmark configuration for better performance

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T14:19:59Z

tardis/visualization/conftest.py

+    # Default to vpackets=0 for performance
+    config.spectrum.virtual.virtual_packet_logging = False
+    config.montecarlo.no_of_virtual_packets = 0
    config.spectrum.num = 2000
    config_verysimple.montecarlo.tracking.track_rpacket = True

-    workflow = StandardTARDISWorkflow(config, enable_virtual_packet_logging=True)
+    workflow = StandardTARDISWorkflow(config, enable_virtual_packet_logging=False)


The workflow_simple_tracked fixture now has virtual packets disabled, but tests in tardis/visualization/tools/tests/test_sdec_plot.py depend on this fixture and attempt to use virtual packet mode. Specifically, test_from_workflow_vs_from_simulation_data_consistency (line 458) asserts that plotter_from_workflow.packet_data["virtual"]["packets_df"] is not None, and test_from_workflow_method_functionality (line 482) calls methods with packets_mode="virtual". These tests will fail because when virtual packets are disabled, the SDECPlotter.from_workflow method does not populate the virtual packet data. Consider either: (1) updating these tests to skip virtual packet mode testing, (2) explicitly enabling virtual packets in the fixture for these specific tests, or (3) creating a separate fixture with virtual packets enabled for tests that need them.

Copilot · 2026-02-17T14:19:59Z

docs/tardis_example.yml

  last_no_of_packets: 1.e+5
-  no_of_virtual_packets: 10
+  # Virtual packets are computationally expensive and only needed for virtual spectrum generation
+  # Set to 0 for better performance; enable in specific tests when needed


The comment mentions "tests" but these configuration files are not test-specific - they are general example/documentation configuration files. The comment should be more general, such as "Set to 0 for better performance; enable when virtual spectrum generation is needed" to better reflect the purpose for users who might reference these example files.

Suggested change

# Set to 0 for better performance; enable in specific tests when needed

# Set to 0 for better performance; enable when virtual spectrum generation is needed

Copilot · 2026-02-17T14:20:00Z

docs/io/optional/tardis_config_logger.yml

  last_no_of_packets: 1.e+4
-  no_of_virtual_packets: 2
+  # Virtual packets are computationally expensive and only needed for virtual spectrum generation
+  # Set to 0 for better performance; enable in specific tests when needed


The comment mentions "tests" but this configuration file is not test-specific - it is a general example/documentation configuration file. The comment should be more general, such as "Set to 0 for better performance; enable when virtual spectrum generation is needed" to better reflect the purpose for users who might reference this example file.

Suggested change

# Set to 0 for better performance; enable in specific tests when needed

# Set to 0 for better performance; enable when virtual spectrum generation is needed

Copilot · 2026-02-17T14:20:00Z

tardis/visualization/conftest.py

+    config.spectrum.virtual.virtual_packet_logging = False
+    config.montecarlo.no_of_virtual_packets = 0
    config.spectrum.num = 2000
    config_verysimple.montecarlo.tracking.track_rpacket = True


This line is using config_verysimple instead of config. The variable config was created at line 45 via deepcopy, but this line incorrectly references the original config_verysimple fixture instead. This means the tracking settings are being applied to the wrong configuration object and won't take effect in the workflow.

Suggested change

config_verysimple.montecarlo.tracking.track_rpacket = True

config.montecarlo.tracking.track_rpacket = True

Copilot · 2026-02-17T14:20:00Z

tardis/visualization/conftest.py

+    # Default to vpackets=0 for performance
+    config_verysimple.spectrum.virtual.virtual_packet_logging = False
+    config_verysimple.montecarlo.no_of_virtual_packets = 0


The simulation_simple_tracked fixture now has virtual packets disabled, but tests in tardis/visualization/tools/tests/test_sdec_plot.py depend on this fixture and attempt to use virtual packet mode. Specifically, test_make_colorbar_labels (line 442) calls _calculate_plotting_data with packets_mode="virtual", and test_from_workflow_vs_from_simulation_data_consistency (line 458) asserts that plotter.packet_data["virtual"]["packets_df"] is not None. These tests will fail because when virtual packets are disabled, the SDECPlotter.from_simulation method does not populate the virtual packet data. Consider either: (1) updating these tests to skip virtual packet mode testing, (2) explicitly enabling virtual packets in the fixture for these specific tests, or (3) creating a separate fixture with virtual packets enabled for tests that need them.

tardis-bot · 2026-02-17T14:30:47Z

*beep* *bop*

Hi, human.

The docs workflow has failed ❌

Click here to see the build log.

…nfigurations

Disable virtual packets by default

66c3835

Copilot AI review requested due to automatic review settings February 17, 2026 14:15

Copilot started reviewing on behalf of wkerzendorf February 17, 2026 14:16 View session

andrewfullard requested changes Feb 17, 2026

View reviewed changes

wkerzendorf marked this pull request as draft February 17, 2026 14:18

Copilot AI reviewed Feb 17, 2026

View reviewed changes

wkerzendorf added 2 commits February 17, 2026 09:40

Set default number of virtual packets to 10 for tardis example

7e94dcc

Add support for virtual packets in SDEC plotter tests and refactor co…

8271030

…nfigurations

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

Disable virtual packets by default#3437

Disable virtual packets by default#3437
wkerzendorf wants to merge 3 commits intotardis-sn:masterfrom
wkerzendorf:restructure/mc/excise_vpacket_test

wkerzendorf commented Feb 17, 2026

Uh oh!

Uh oh!

Uh oh!

andrewfullard Feb 17, 2026

Uh oh!

Uh oh!

tardis-bot commented Feb 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

tardis-bot commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# Set to 0 for better performance; enable in specific tests when needed
	# Set to 0 for better performance; enable when virtual spectrum generation is needed

	config_verysimple.montecarlo.tracking.track_rpacket = True
	config.montecarlo.tracking.track_rpacket = True

Uh oh!

Comments

Conversation

wkerzendorf commented Feb 17, 2026

Uh oh!

Uh oh!

Uh oh!

andrewfullard Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tardis-bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

tardis-bot commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tardis-bot commented Feb 17, 2026 •

edited

Loading