Xinyue/testapp p2 by xinyue-uoft · Pull Request #38 · DUNE-DAQ/tpglibs

xinyue-uoft · 2025-12-02T03:17:29Z

Description

See issue #21 for motivation and overview.

This PR adds a standalone TPGenerator test application and the supporting test infrastructure needed to validate TPGenerator algorithms offline, independently of the full DAQ system. The new test app follows the same pattern as the existing test_tpg_processor_app.cxx (single processor test application), but is focused on end‑to‑end verification of TPGenerator using frame format data and binary validation files.

Main changes:
• New TPGenerator test application (test_tpg_generator_app.cxx)
• Implements a CLI test app (installed as tpglibs_generator_test) that:
• Reads binary input frames from a .bin file using FrameReader.
• Adapts them to DUMMY_FRAME_STRUCT via DummyFrameAdapter so they are compatible with TPGenerator.
• Configures TPGenerator from a JSON config (processor_configs, channel_plane_mappings, test_config)(same format as actual config)
• Runs frames through TPGenerator and collects output TriggerPrimitives.
• (Can also turn off optionally) validates the output against a .val validation file using TPReader + TPComparator.
• BinaryFormatSpecification.md and example README are updated to describe the TP binary format, header, sort order, and concrete usage of the new test app and datasets.

Type of change

Documentation (non-breaking change that adds or improves the documentation)
New feature or enhancement (non-breaking change which adds functionality)
Optimization (non-breaking change that improves code/performance)
Bug fix (non-breaking change which fixes an issue)
Breaking change (whatever its nature)

Testing checklist

Unit tests pass (e.g. dbt-build --unittest)
Minimal system quicktest passes (pytest -s minimal_system_quick_test.py)
Full set of integration tests pass (daqsystemtest_integtest_bundle.sh)
Python tests pass if applicable (e.g. python -m pytest)
Pre-commit hooks run successfully if applicable (e.g. pre-commit run --all-files)

This is a completely standalone test application that does not interfere with DAQ software. Test is mainly done by unit tests directed to the test application infrastructure. Two simple test cases are developed and tested, where the test application behaves as expected: 1. an all zero input sanity check, no tp 2. a square wave in a single adc channel, with tp also localized to single channel.

Further checks

Code is commented where needed, particularly in hard-to-understand areas
Code style is correct (dbt-build --lint, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)
If applicable, new tests have been added or an issue has been opened to tackle that in the future.
(Indicate issue here: # (issue))

…ests.

…test frame reading infrastructure.

…pplication initial version. Change dummy_frame_struct structure to directly use TPGenerator default format, yields a simple and clear data flow from binaries to tpgenerator.

…oth the TPG Processor test application and the TPGenerator test application.

xinyue-uoft · 2025-12-02T03:20:04Z

Eventually plan is to add the binary files and config for the two sanity type tests to DUNE asset system. To allow reviewers to perform independent tests for this PR, I will post them here together with their generation script soon.

xinyue-uoft · 2025-12-05T09:29:56Z

I have temporarily included the two test binary + config + validation file generation script for the following three test cases. We can remove them after review if they shouldn't go into tpglibs.

Test Patterns and Expected TP Presence

This summarizes which frames/channels are expected to have TPs for the sanity, multiframe, and golden test.

1. Sanity test (`tpg_generator_sanity_*`)

Input: All ADC values are below threshold.

Frame	Channels with TP	Notes
0	none	No threshold crossings

2. Multi-frame test (`tpg_generator_multiframe_*`)

Frame	Channels with TP	Notes
0	ch 0	Single spike region in channel 0
1	none	All zeros
2	ch 16	Single spike region in channel 16
3	none	All zeros
4	ch 0, ch 16, ch 32, ch 48	Four independent spike regions

Each spike region is contiguous within one channel, so each listed channel generates exactly one TP.

3. Golden test (`tpg_generator_golden_*`)

Frame	Channels with TP	Notes
0	ch 0	Single spike region in channel 0
1	none	All zeros

aeoranday

I have not tested the functionality of this PR yet. This is only a result of reading with various style and design comments, questions, and suggestions.

One major comment is the header-only design of the test application files. Since none of the classes are general and require header-only, I would suggest that these get separated into the include–source structure.

I'll test after these changes. One aspect I'm concerned about is the 16 b dynamic range in the dummy frame which does not match the expected 14 b signal. Since most of the applied data is composed of 0s, I would believe these "passed", but I believe high-valued testing would start to break.

aeoranday · 2025-12-09T12:44:08Z

docs/testapp/BinaryFormatSpecification.md


-## 3. Test File Examples
+**Key Fields in TriggerPrimitive** (for reference):
+- `time_start` (int64_t): Timestamp when TP starts


Suggested change

- `time_start` (int64_t): Timestamp when TP starts

- `time_start` (uint64_t): Timestamp when TP starts

docs/testapp/BinaryFormatSpecification.md

src/BinaryFileValidator.cpp

aeoranday · 2025-12-09T13:43:20Z

include/tpglibs/testapp/common/BinaryFileValidator.hpp

+class BinaryFileValidator {
+ public:
+  static constexpr uint32_t MAGIC_NUMBER = 0x54504754;  ///< "TPGT"
+  static constexpr uint32_t VERSION = 0x010004;         ///< 1.0.4 in hex


Where is this version derived from? Is this the tpglibs version at the time of this creation? We cannot keep this number up-to-date with the tpglibs version.

This is indeed the version at creation. The hope is to create a marker for backward compatibility. The original goal is that in the future if there is a breaking change to the format of the binary files, then we update this value here to mark the end of compatibility with previously generated files.

Maybe we can just keep this independent of tpglibs version?

aeoranday · 2025-12-10T13:11:51Z

test/apps/generate_golden_test_data.cxx

+ // Amplitude: threshold + 100
+ constexpr int16_t SPIKE_ADC_DELTA_ABOVE_THRESHOLD = 100;
+
+ // TPGenerator detects the TP when processing t = 106 in this setup


TPGenerator generates the TP at t = 106. It detects it starting at t = 100 in this case.

aeoranday · 2025-12-10T13:25:03Z

test/apps/generate_golden_test_data.cxx

+  * @brief Calculate absolute timestamp for TP time_start
+  * Formula matches TPGenerator: (t - samples_over_threshold) * sample_tick_difference + timestamp


Based on this description, this test verifies that the TP generated is equivalent to an expected TP.

However, the objective for creating these tests seems to want to validate that the generated TP is expected for the known signal. The TP's time_start should be SPIKE_START_SAMPLE * SAMPLE_TICK_DIFFERENCE + frame_timestamp since we want to know that we are accurately representing the injected signal that we control.

Since the TPGenerator (in the current state) is consistent between verification and validation, the comparison appears the same (at the moment), but we should really be aiming for the second case.

aeoranday · 2025-12-10T13:30:32Z

docs/testapp/BinaryFormatSpecification.md

+┌─────────────────────┐
+│ Frame Header        │ (16 bytes)
+│ - timestamp (8)     │ uint64_t: Frame timestamp
+│ - another_key (8)   │ uint64_t: Additional header field


Why do we have this? I see later that you set it to an arbitrary value, so it seems like this is not necessary for our purposes. I'm aware the reference has it, but the value is relevant for them.

Indeed it is not really used here. It is purely carried over from the reference. Will be removed.

unittest/binary_file_validator_test.cxx

unittest/include/test_helpers.hpp

aeoranday

(Missed this on previous review submission.)

…ping and warnings.

…tion of `channel_t` and detailing the width of the TP channel value.

…d version. This improves debugging by displaying actual values encountered.

…lper.

…larity. Renamed compare_single_tp() to compare() to fix counterintuitive naming.

… on first. This provides complete diagnostic information without repeated test runs.

alessandrothea

Great work.
I did not have the time to look into the code.
The comments about naming conventions: in general I would trade the use of dummy and test words for something more specific to the purpose of the application, which is the validation of the tpg chain by regression tests involving generation and comparison to reference patterns.

alessandrothea · 2025-12-17T08:19:48Z

include/tpglibs/testapp/frame/DummyFrameAdapter.hpp

+  static constexpr int16_t s_adc_min_value = 0;       // 0 - 14-bit min
+
+  uint64_t timestamp;      ///< Frame timestamp (8 bytes)
+  uint64_t another_key;    ///< Additional header field (8 bytes)


another_key seems a bit random. Something likeheader is more to the point

This was mentioned by Alejandro above. It is a carry over artifact from the original dummy frame definition https://github.com/DUNE-DAQ/datahandlinglibs/blob/develop/include/datahandlinglibs/ReadoutTypes.hpp that was referenced. Will be removed soon.

alessandrothea · 2025-12-17T08:22:47Z

CMakeLists.txt

+daq_add_application(tpglibs_generator_test test_tpg_generator_app.cxx TEST LINK_LIBRARIES tpglibs)
+
+# Add golden test data generator utility
+daq_add_application(generate_golden_test_data generate_golden_test_data.cxx TEST LINK_LIBRARIES tpglibs)


"Golden test data" is a little, how to say, mysterious.
What is golden? What test?
I would rather talk about reference + data + patterns

Yes. I am planning to remove these generate test scripts before merging this PR. It is better to include the tests in asset system directly.

alessandrothea · 2025-12-17T08:23:22Z

CMakeLists.txt

+daq_add_application(generate_golden_test_data generate_golden_test_data.cxx TEST LINK_LIBRARIES tpglibs)
+
+# Add example test files generator utility
+daq_add_application(generate_example_tests generate_example_tests.cxx TEST LINK_LIBRARIES tpglibs)


What does this app generate? tests or test data?

As explained above, these are temporary.

alessandrothea · 2025-12-17T08:25:01Z

include/tpglibs/testapp/frame/DummyFrameAdapter.hpp

+namespace testapp {
+
+/**
+ * @brief Dummy frame structure for test applications


It seems to me that this is more than a "dummy" frame: this is a reference/minimal 14b dataframe.

Yes. The naming is more or less carried on from datahandlinglibs calling theirs DUMMY_FRAME_STRUCT.

alessandrothea · 2025-12-17T09:16:28Z

This is a summary of my comment at yesterday's Readout WG Meeting:

inspection of the generated TPs and their analysis (potentially) is an important aspect for preparation of input patterns and debugging issues when detected by the tool
my suggestion is to add, if not available already, a way to write output products (TPs) in a format easily consumable by analysis tools (e.g. python, pandas). Being able to inspect input waveforms would be a bonus.
Depending on the typical output size, the "analysis" format options could include CSV or HDF5 or...

This enforces DUNE DAQ style guide naming convention for static class data members.

…s matches the actual physical constraint based on frame dimensions (64 channels × 256 time samples).

…6 to 8 bytes.

…mt library was included but never used in active code.

…y was not used in these files.

xinyue-uoft · 2026-01-20T22:44:24Z

Addressed the all the review comments left over.

Regarding the comments related to generate_*_data.cxx: Sorry for the confusion, but these files are not really part of this commit. I am including them here for context about the full data flow. The original plan is to directly have the generated configuration and binary files added to the asset management system.

xinyue-uoft added 11 commits November 18, 2025 21:54

Add FrameReader class for reading binary frame files and associated t…

b6d0b7b

…ests.

Add DummyFrameAdapter and TPReader classes with associated tests for …

fcc6bab

…test frame reading infrastructure.

Add TPComparator module for comparing TPs, and its unittest.

f1208bd

Implement test configuration validation. Implement tpgenerator test a…

e669e2c

…pplication initial version. Change dummy_frame_struct structure to directly use TPGenerator default format, yields a simple and clear data flow from binaries to tpgenerator.

Fixing unittest error after previous refactor commit.

4a842ee

Refactor out test configuration parser functionality.

0afd352

Refactor the generator test applicaiton

ba7d367

Add max_frames consistency validation to test application,

50de14a

Added some comments.

fc33a12

Update the BinaryFormatSpecification file to contain information on b…

a18b531

…oth the TPG Processor test application and the TPGenerator test application.

Improving documentation.

a1fdb11

xinyue-uoft self-assigned this Dec 2, 2025

xinyue-uoft added the enhancement New feature or request label Dec 2, 2025

xinyue-uoft linked an issue Dec 2, 2025 that may be closed by this pull request

Write A Basic Test Application #21

Closed

(Could be temporary) include test generation scripts

fa60ce2

xinyue-uoft requested a review from aeoranday December 5, 2025 09:30

aeoranday reviewed Dec 10, 2025

View reviewed changes

aeoranday requested changes Dec 10, 2025

View reviewed changes

xinyue-uoft added 10 commits December 12, 2025 10:49

convert testapp classes from header-only to separate source

874b660

Constrain dummy frame ADC values to 14-bit range [0, 16383] with clam…

bb0b3eb

…ping and warnings.

Update BinaryFormatSpecification documentation: clarifying the defini…

9247ee5

…tion of `channel_t` and detailing the width of the TP channel value.

Fix eof condition.

25fd409

naming convention update.

5ed5f25

Error messages now show expected and found values for magic number an…

354a1f6

…d version. This improves debugging by displaying actual values encountered.

Refactored parse_tpgenerator_config to use validate_required_array he…

a3f4cc9

…lper.

Renamed FrameReadStatus enum values to use k prefix.

fb2afa3

Renamed TPComparator to TPValidator and compare() to validate() for c…

30fc93a

…larity. Renamed compare_single_tp() to compare() to fix counterintuitive naming.

Moved size mismatch check before sorting in TPValidator.

0d23f17

xinyue-uoft added 3 commits December 15, 2025 13:11

Replaced manual comparison logic with std::tie in TPValidator sort_tps.

0fe304b

TPValidator::compare now collects all mismatches instead of returning…

b4d1b90

… on first. This provides complete diagnostic information without repeated test runs.

Renamed TPReader to TPValidationReader.

5ca4adc

alessandrothea requested changes Dec 17, 2025

View reviewed changes

xinyue-uoft added 6 commits January 7, 2026 23:40

Rename static constexpr members in TPValidationReader to use s_ prefix.

290e161

This enforces DUNE DAQ style guide naming convention for static class data members.

Reduced maximum TP count validation limit from 1 million to 8192. Thi…

3f6cb0e

…s matches the actual physical constraint based on frame dimensions (64 channels × 256 time samples).

Remove unused another_key field from frame header, reducing it from 1…

c2ee7ec

…6 to 8 bytes.

Remove unused fmt library includes and defines from unit tests. The f…

cf38955

…mt library was included but never used in active code.

Move test_help.hpp to include directory.

9bff8b5

Remove unused FMT_HEADER_ONLY defines from unit tests. The fmt librar…

65977af

…y was not used in these files.

xinyue-uoft requested a review from aeoranday January 20, 2026 22:44

	- `time_start` (int64_t): Timestamp when TP starts
	- `time_start` (uint64_t): Timestamp when TP starts

		* @brief Calculate absolute timestamp for TP time_start
		* Formula matches TPGenerator: (t - samples_over_threshold) * sample_tick_difference + timestamp

Conversation

xinyue-uoft commented Dec 2, 2025

Description

Type of change

Testing checklist

Further checks

Uh oh!

xinyue-uoft commented Dec 2, 2025

Uh oh!

xinyue-uoft commented Dec 5, 2025

Test Patterns and Expected TP Presence

1. Sanity test (tpg_generator_sanity_*)

2. Multi-frame test (tpg_generator_multiframe_*)

3. Golden test (tpg_generator_golden_*)

Uh oh!

aeoranday left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xinyue-uoft Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aeoranday left a comment

Choose a reason for hiding this comment

Uh oh!

alessandrothea left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xinyue-uoft Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alessandrothea commented Dec 17, 2025

Uh oh!

xinyue-uoft commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

1. Sanity test (`tpg_generator_sanity_*`)

2. Multi-frame test (`tpg_generator_multiframe_*`)

3. Golden test (`tpg_generator_golden_*`)

xinyue-uoft Jan 7, 2026 •

edited

Loading

xinyue-uoft Jan 7, 2026 •

edited

Loading