Skip to content

Conversation

@alessandrothea
Copy link
Contributor

@alessandrothea alessandrothea commented Oct 21, 2025

Address the RTCM limitations in #415.
The candidate time is shifted by candidate_offset_ts (signed integer, leaving the option to inject candidates into the future if needed), and the window is define by candidate_window_before_ts/candidate_window_after_ts

The use of TCReadoutMap for the window definition is discontinued (since it belongs to a different domain)

Requires associated changes in appmodel
DUNE-DAQ/appmodel#259

@alessandrothea alessandrothea changed the base branch from develop to patch/fddaq-v5.4.x October 27, 2025 10:57
@alessandrothea alessandrothea changed the base branch from patch/fddaq-v5.4.x to develop November 12, 2025 15:16
@alessandrothea alessandrothea marked this pull request as ready for review November 12, 2025 15:16
Copy link
Contributor

@MRiganSUSX MRiganSUSX left a comment

Choose a reason for hiding this comment

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

Hi @alessandrothea. Thanks for this improvement.

My 2 cents:

  • I'd suggesting removing the comments that are there for old version of the code. I find it more confusing and if anyone wants to go back we have github with versioning and older releases
  • adding the m_tcout_time_offset_ts to the TLOG section mentioning what the settings used are
  • one place that is a clear potential area for problems is the candidate.time_candidate = timestamp + m_tcout_time_offset_ts, specifically the sign. We should make sure to explain / document that the offset value should be negative (in most use cases) as the offset itself is applied as a +.
  • for the comments, such as /// @brief Output candidate end time, based off trigger timestamp would using term relative make more sense?

@alessandrothea
Copy link
Contributor Author

Thanks for the comments, @MRiganSUSX
I've removed the commented code, fixed the comments and added the TLOG.
Excellent point about the offset sign: I changed the sign in the candidate central time calculation and the parameter name to candidate_backshift_ts. Hopefully this conveys the purpose of the parameter, together with the description in the schema.

@MRiganSUSX MRiganSUSX changed the base branch from develop to prep-release/fddaq-v5.5.0 November 21, 2025 16:15
@MRiganSUSX MRiganSUSX self-requested a review November 21, 2025 16:16
Copy link
Contributor

@MRiganSUSX MRiganSUSX left a comment

Choose a reason for hiding this comment

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

Thanks @alessandrothea for this improved functionality, and for addressing my concerns.

I can confirm that I see Random TC Maker load correct configuration, generate TCs as expected (shifted time_candidate, window around that), and that this is reflected in the MLT and in the readout request that is made.

config:
Screenshot from 2025-11-21 16-14-03

loaded config:
Screenshot from 2025-11-21 16-24-14

logs:

2025-Nov-21 16:18:51,431 DEBUG_1 [void dunedaq::trigger::RandomTCMakerModule::send_trigger_candidates() at /nfs/home/mrigan/dune-daq/fddaq-v5.5.0-rc2-a9/sourcecode/trigger/plugins/RandomTCMakerModule.cpp:278] random-tc-generator at timestamp 110233645714503944, pushing a candidate with timestamp 110233645714502944
2025-Nov-21 16:18:51,432 DEBUG_3 [void dunedaq::trigger::TCProcessor::make_td(const dunedaq::trigger::TCWrapper*) at /nfs/home/mrigan/dune-daq/fddaq-v5.5.0-rc2-a9/sourcecode/trigger/src/TCProcessor.cpp:268] Got TC of type 4, timestamp 110233645714502944, start/end 110233645714501944/110233645714503945
2025-Nov-21 16:18:51,432 DEBUG_3 [dunedaq::dfmessages::TriggerDecision dunedaq::trigger::TCProcessor::create_decision(const PendingTD&) at /nfs/home/mrigan/dune-daq/fddaq-v5.5.0-rc2-a9/sourcecode/trigger/src/TCProcessor.cpp:319] , TC detid: , TC type: 4, TC cont number: 1, DECISION trigger type: 16, DECISION timestamp: 110233645714502944, request window begin: 110233645714501944, request window end: 110233645714503945

So the custom test was successful.
However it seems the removal of the readout map was not propagated to all places where it is required still, breaking multiple integration tests :

ERROR readout_type_scan_test.py::test_nanorc_success[DAPHNE_Stream_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_log_files[DAPHNE_Stream_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_data_files[DAPHNE_Stream_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_nanorc_success[DAPHNE_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_log_files[DAPHNE_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_data_files[DAPHNE_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_nanorc_success[DAPHNE_TPG_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_log_files[DAPHNE_TPG_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found
ERROR readout_type_scan_test.py::test_data_files[DAPHNE_TPG_System-run_nanorc0] - RuntimeError: object "def-random-readout@TCReadoutMap" is not found

.

@alessandrothea
Copy link
Contributor Author

Hi @MRiganSUSX , thanks for checking.
I suspect the issue is that the readout map entry I removed is also used in the mlt for converting the Random TC into a TD.
Let me put it back and try again.

@alessandrothea
Copy link
Contributor Author

After the re-introduction of def-random-readout, the readout_type_scan_test.py has "improved". The remaining errors don't seem related to the RTCM anymore, at the first order.

=========================== short test summary info ============================
FAILED sourcecode/daqsystemtest/integtest/readout_type_scan_test.py::test_data_files[DAPHNE_Stream_System-run_nanorc0]
FAILED sourcecode/daqsystemtest/integtest/readout_type_scan_test.py::test_data_files[DAPHNE_System-run_nanorc0]
FAILED sourcecode/daqsystemtest/integtest/readout_type_scan_test.py::test_data_files[DAPHNE_TPG_System-run_nanorc0]
=================== 3 failed, 24 passed in 401.82s (0:06:41) ===================```

@MRiganSUSX
Copy link
Contributor

I reran the integtest bundle, and can confirm that the previous errors are no longer there (regarding the map).

Just for completeness, there are still some errors observed:

from readout_type_scan_test:

 🚨 DAPHNEStream fragment for SrcID Detector_Readout_0x00000001 in record (9, 0) has size 15176 (outside range [451658, 470538]) 🚨
 🚨 DAPHNEStream fragment for SrcID Detector_Readout_0x00000000 in record (10, 0) has size 15648 (outside range [451658, 470538]) 🚨
 🚨 DAPHNEStream fragment for SrcID Detector_Readout_0x00000001 in record (10, 0) has size 15176 (outside range [451658, 470538]) 🚨

from fake_data_producer_test:

 🚨 WIBEth fragment for SrcID Detector_Readout_0x00000000 in record (1, 0) has size 7272 (outside range [14472, 21672]) 🚨
 🚨 WIBEth fragment for SrcID Detector_Readout_0x00000001 in record (1, 0) has size 7272 (outside range [14472, 21672]) 🚨
 🚨 WIBEth fragment for SrcID Detector_Readout_0x00000001 in record (2, 0) has size 7272 (outside range [14472, 21672]) 🚨
 🚨 WIBEth fragment for SrcID Detector_Readout_0x00000000 in record (2, 0) has size 7272 (outside range [14472, 21672]) 🚨

but seem unrelated to this code change.

@MRiganSUSX MRiganSUSX self-requested a review November 25, 2025 14:07
@wesketchum wesketchum merged commit 72fd44a into prep-release/fddaq-v5.5.0 Nov 25, 2025
3 checks passed
@wesketchum wesketchum deleted the thea/random_tc_config branch November 25, 2025 14:34
@wesketchum wesketchum restored the thea/random_tc_config branch November 25, 2025 18:33
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