Skip to content

Conversation

@wkerzendorf
Copy link
Member

downgrade the HE workflow tests

Copilot AI review requested due to automatic review settings August 2, 2025 18:09
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 reduces the computational requirements for high energy (HE) workflow tests by decreasing test parameters to make regression testing faster and more efficient.

  • Reduced number of packets from 300,000 to 30,000 (10x decrease)
  • Decreased time steps from 500 to 20 (25x decrease)
Comments suppressed due to low confidence (2)

tardis/workflows/high_energy/tests/test_tardis_he_workflow.py:18

  • Reducing the number of packets from 3e5 to 3e4 (10x decrease) may significantly impact test coverage and the statistical reliability of the high energy workflow tests. Consider whether this reduction still provides adequate coverage for detecting regressions in the workflow behavior.
        "number_of_packets": int(3e4),

tardis/workflows/high_energy/tests/test_tardis_he_workflow.py:19

  • Reducing time steps from 500 to 20 (25x decrease) may not adequately test the temporal evolution aspects of the high energy workflow. This dramatic reduction could miss time-dependent bugs or convergence issues that only manifest over longer simulation periods.
        "time_steps": 20,

@tardis-bot
Copy link
Contributor

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

Details

Complete output(might be large):

Details
All checks passed!

@tardis-bot
Copy link
Contributor

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

Significantly changed benchmarks:

Details

All benchmarks:

Details
Benchmarks that have stayed the same:

| Change   | Before [ef0e64d6] <master>   | After [2328ee0d]    | Ratio   | Benchmark (Parameter)                                                                                                               |
|----------|------------------------------|---------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------|
|          | 45.2±40μs                    | 66.5±30μs           | ~1.47   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_scatter                                          |
|          | 2.91±0.4μs                   | 3.42±0.6μs          | ~1.18   | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_bad_vpacket                                       |
|          | 45.5±30μs                    | 50.1±30μs           | ~1.10   | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_line_emission                                         |
|          | 1.75±0.3μs                   | 1.49±0.6μs          | ~0.85   | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_line                    |
|          | 32.8±10μs                    | 27.8±8μs            | ~0.85   | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_last_interaction_tracker_list |
|          | 6.07±1μs                     | 6.60±0.6μs          | 1.09    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket                                           |
|          | 551±100ns                    | 570±200ns           | 1.03    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_photoabsorption_opacity_calculation                              |
|          | 38.4±0.02μs                  | 39.6±0.4μs          | 1.03    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_generate_rpacket_tracker_list                  |
|          | 662±0.2ns                    | 677±1ns             | 1.02    | transport_montecarlo_interaction.BenchmarkTransportMontecarloInteraction.time_thomson_scatter                                       |
|          | 38.8±0.01s                   | 39.2±0.04s          | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis                                                                                       |
|          | 1.01±0m                      | 1.02±0m             | 1.01    | run_tardis.BenchmarkRunTardis.time_run_tardis_rpacket_tracking                                                                      |
|          | 190±0.4ns                    | 193±0.6ns           | 1.01    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_intensity_black_body                                       |
|          | 2.11±0m                      | 2.11±0m             | 1.00    | spectrum_formal_integral.BenchmarkTransportMontecarloFormalIntegral.time_FormalIntegrator_functions                                 |
|          | 7.14±3μs                     | 7.13±2μs            | 1.00    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_volley                                    |
|          | 1.20±0μs                     | 1.20±0μs            | 0.99    | transport_geometry_calculate_distances.BenchmarkTransportGeometryCalculateDistances.time_calculate_distance_boundary                |
|          | 3.00±0.3ms                   | 2.97±0.4ms          | 0.99    | transport_montecarlo_single_packet_loop.BenchmarkTransportMontecarloSinglePacketLoop.time_single_packet_loop                        |
|          | 2.83±0ms                     | 2.79±0.02ms         | 0.98    | transport_montecarlo_main_loop.BenchmarkTransportMontecarloMontecarloMainLoop.time_montecarlo_main_loop                             |
|          | 3.49±0.5μs                   | 3.36±0.5μs          | 0.96    | transport_montecarlo_vpacket.BenchmarkMontecarloMontecarloNumbaVpacket.time_trace_vpacket_within_shell                              |
|          | 3.72±0.02ms                  | 3.53±0ms            | 0.95    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('scatter')                                     |
|          | 521±200ns                    | 491±200ns           | 0.94    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_compton_opacity_calculation                                      |
|          | 4.98±0.08ms                  | 4.70±0.05ms         | 0.94    | opacities_opacity_state.BenchmarkOpacitiesOpacityState.time_opacity_state_initialize('macroatom')                                   |
|          | 62.7±0.1ms                   | 58.8±0.08ms         | 0.94    | transport_montecarlo_packet_trackers.BenchmarkTransportMontecarloPacketTrackers.time_rpacket_trackers_to_dataframe                  |
|          | 551±200ns                    | 511±200ns           | 0.93    | opacities_opacity.BenchmarkMontecarloMontecarloNumbaOpacities.time_pair_creation_opacity_calculation                                |
|          | 2.65±1μs                     | 2.47±1μs            | 0.93    | transport_montecarlo_estimators_radfield_estimator_calcs.BenchmarkMontecarloMontecarloNumbaPacket.time_update_line_estimators       |

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

@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 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 cb55acf into tardis-sn:master Aug 6, 2025
41 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants