Added some utility functions for, and documentation about, trigger types used in integtests#161
Added some utility functions for, and documentation about, trigger types used in integtests#161bieryAtFnal wants to merge 27 commits into
Conversation
Updated the survey document to include an introduction and a summary table for integration tests.
…formation in tables.
Updated the integtest documentation to include WIB counts and trigger rates for various tests.
…/DUNE-DAQ/integrationtest into kbiery/integtest_trigger_choices
Updated descriptions of trigger sources and their configurations in the integration tests documentation.
MRiganSUSX
left a comment
There was a problem hiding this comment.
Hi @bieryAtFnal,
Thanks for this updated, much cleaner, functionality.
I'm leaving several suggestions here and across the linked 5 PRs.
Don't think there are any glaring problems, some small style things and some places that could be tidied up.
| # exists in the input/template DUNE-DAQ configuration, so there is the possibility of some | ||
| # confusion if a kRandom TCReadoutMap entry exists. | ||
| # ************************************************************ | ||
| def set_RTCM_trigger_params(integtest_params: data_classes.integtest_param_base_class, trigger_rate: float = None, |
There was a problem hiding this comment.
Should we be consistent here and keep the snake_case, ie set_rtcm_trigger_params ?
(It also breaks PEP 8 but I'm unsure whether we are adhering to that in out python repos)
If you agree, this will need to be addressed across the linked PRs as well.
| def set_fake_hsi_trigger_params(integtest_params: data_classes.integtest_param_base_class, trigger_rate: float = None, | ||
| readout_window_before_ticks: int = None, readout_window_after_ticks: int = None): | ||
| if trigger_rate is not None: | ||
| integtest_params.config_substitutions.append( | ||
| data_classes.attribute_substitution( | ||
| obj_class="FakeHSIEventGeneratorConf", | ||
| updates={"trigger_rate": trigger_rate}, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This is not immediately problematic, but the old places this is replacing (tpreplay_test.py, trigger_bitwords_test.py) were more specific in targeting obj_id="fakehsi".
If we ever have a situation where we have multiple FakeHSIEventGeneratorConf (not sure it is a concern), all would be affected.
The suggestion would be to add an optional obj_id: str = "*" so one can be more specific if needed, with the default doing what it is doing now.
| def enable_fake_hsi_trigger(integtest_params: data_classes.integtest_param_base_class, trigger_rate: float = None, | ||
| readout_window_before_ticks: int = None, readout_window_after_ticks: int = None): | ||
| if not isinstance(integtest_params, data_classes.integtest_params_for_generated_dunedaq_config): | ||
| fail_msg = "The FakeHSI trigger can only be dynamically enabled in DUNE-DAQ configurations that are generated by the integration test infrastructure, not ones that are pre-defined." |
There was a problem hiding this comment.
189 chars is above most PEP8 string limits.
If you'd like, this could be split across multiple lines, ie:
fail_msg = ("The FakeHSI trigger can only be dynamically enabled in DUNE-DAQ "
"configurations that are generated by the integration test infrastructure, "
"not ones that are pre-defined.")
| except IsADirectoryError: | ||
| print(f"File {file} could not be deleted because it is actually a directory.") | ||
|
|
||
| # ************************************************************ |
There was a problem hiding this comment.
Most linters require 2 empty lines before any new def, as is the case earlier in this file.
| integtest_params.fake_hsi_enabled = True | ||
| set_fake_hsi_trigger_params(integtest_params, trigger_rate, readout_window_before_ticks, readout_window_after_ticks) | ||
|
|
||
| # ************************************************************ |
There was a problem hiding this comment.
Most linters require 2 empty lines before any new def, as is the case earlier in this file.
| integtest_params.config_substitutions.append( | ||
| data_classes.attribute_substitution( | ||
| obj_class="TCReadoutMap", | ||
| obj_id = "def-hsi-tc-map", |
There was a problem hiding this comment.
There should be no empty spaces around = in keyword arguments,
ie suggesting changing to obj_id="def-hsi-tc-map".
| integtest_params.config_substitutions.append( | ||
| data_classes.attribute_substitution( | ||
| obj_class="RandomTCMakerConf", | ||
| obj_id = "random-tc-generator", |
There was a problem hiding this comment.
Same as before, empty spaces around =:
obj_id="random-tc-generator",
| @@ -0,0 +1,75 @@ | |||
| # Survey of trigger and readout window configurations that are used in existing integtests, Summer 2026 | |||
There was a problem hiding this comment.
This documentation is GREAT.
My only concern is whether it makes sense to have the year in the filename instead (completely ok in the text), as updates later on can cause link breaks, or the file may look like it obsolete later on.
| | long_window_readout_test.py | 0.05 Hz trigger rate, RWBT=-100000000, RWET=1000000, RWW=1.62 sec | - | - | | ||
| | minimal_system_quick_test.py | 1 Hz trigger rate, RWBT=-2000, RWET=5, RWW=32.1 usec | - | - | | ||
| | readout_type_scan_test.py | 1 Hz trigger rate, various readout window widths | - | various rates and readout window widths | | ||
| | sample_ehn1_multihost_test.py | - | - | - | |
There was a problem hiding this comment.
For a person not reading the whole documentation, it may suggest this test has no triggers.
Maybe we should add a note or legend distinguishing "set by integtest" from "inherited from predefined config"; or add a brief entry such as "1 Hz RTCM + 3 Hz FakeHSI (from predefined config local-1x1-config)".
|
|
||
| ### Other integtests: | ||
|
|
||
| | Integtest name | RTCM | FakeHSI | Triggers from TPs | |
There was a problem hiding this comment.
Please remember to open an issue for these parts that need updating.
Updated integration test details for trigger rates and configurations, including specific parameters for WIB and DAPHNE data. Clarified expected rates and added observed rates for various tests.
Description
It is not always easy to determine which trigger type(s) are used in which integtests, nor how to specify the parameters for a given trigger type in a new integtest.
The changes in this PR attempt to help with that, and they provide several utility functions that can be used to enable Fake HSI triggers and/or set basic parameters associated with FakeHSI and/or RandomTriggerCandidateMaker triggers. This PR also include some preliminary documentation about which triggers are used in which integtests.
This PR is correlated with ones in several other repos:
To test these changes, the following steps can be used:
Type of change
Testing checklist
dunedaq_integtest_bundle.sh)Further checks