Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New DAQ source for L1Trigger scouting #43467

Merged
merged 21 commits into from
Dec 19, 2023

Conversation

Mmiglio
Copy link
Contributor

@Mmiglio Mmiglio commented Dec 1, 2023

PR description:

This PR introduces a new DAQ source for the L1Trigger Scouting Run3 demonstrator.
The L1 Data Scouting (DS) system operates in parallel with respect to the central DAQ. Therefore, L1 DS data are not inserted in the CMS event data, ensuring complete Independence of the two systems.
By design, L1 Scouting data are collected and processed based on the orbit counter. Hence, one CMSSW event correspond to data collected during one LHC orbit (3564 BX).
An overview of this system can be found in this presentation.

The main components introduced in this PR include:

  • A DAQSourceModel that reads data from all scouting sources and merges them based on the Orbit counter.
  • Unpackers for the two sources currently tested during Run3 (uGMT and Calo DeMux).
  • Data formats for the scouting objects.

PR validation:

The system has been tested from two perspectives:

  • Verified that events collected in the Zero Bias dataset match the raw data collected and unpacked from the L1DS.
  • Tested online data tacking using DAQ3VAL resources (scout-rubu -> d3v-fu (filter units) -> d3v lustre + monitoring).

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

No backport needed.

Implemented a new DAQSourceModel to read and merge Run3 L1 scouting data.
- Similar to the existing FRDStriped input source
- Modified EvFDaqDirector to enable the possibility of having multiple data sources per ramdisk
- Added DataFormats/L1Scouting containing Scouting Raw Data Collection
  - Vector of FEDRawData
  - Included optional argument wordSize in the FEDRawData resize methods: 8 bytes by default, can be set to 4 bytes for scouting
  - Not a real FED, using a SRDCollection since L1scouting data will never be mixed with event data
- Removed old scouting files that were used as an example
- Test script for the ScoutingRun3 daq source
Implemented a new DAQSourceModel to read and merge Run3 L1T scouting raw data.
- Similar to the existing FRDStriped input source
- Modified EvFDaqDirector to enable the possibility of having multiple data sources per ramdisk
- Added DataFormats/L1Scouting containing Scouting Raw Data Collection
  - Vector of FEDRawData
  - Included optional argument wordSize in the FEDRawData resize methods: 8 bytes by default, can be set to 4 bytes for scouting
  - Not a real FED, using a SRDCollection since L1scouting data will never be mixed with event data
- Removed old scouting files that were used as an example

Added first two unpackers for uGMT and CaloL2 data
- Custom data format, similar to l1t objects, but more efficent in term of storage and write speed.
  - Save 30% in storage
- Objects stored in an OrbitCollection

Example of dummy analyzer used to dump scouting data
- consumes collections produced by unpackers and dump them to screen
@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43467/38020

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

A new Pull Request was created by @Mmiglio (Matteo Migliorini) for master.

It involves the following packages:

  • DataFormats/FEDRawData (daq)
  • DataFormats/L1Scouting (****)
  • DataFormats/L1ScoutingRawData (****)
  • EventFilter/L1ScoutingRawToDigi (****)
  • EventFilter/Utilities (daq)
  • L1TriggerScouting/Utilities (****)

The following packages do not have a category, yet:

DataFormats/L1Scouting
DataFormats/L1ScoutingRawData
EventFilter/L1ScoutingRawToDigi
L1TriggerScouting/Utilities
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@smorovic, @emeschi, @cmsbuild can you please review it and eventually sign? Thanks.
@missirol, @rovere, @Martin-Grunewald this is something you requested to watch as well.
@rappoccio, @antoniovilela, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@smorovic
Copy link
Contributor

smorovic commented Dec 1, 2023

@cmsbuild please test with #43461

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c4eca/36241/summary.html
COMMIT: 9dd2a02
CMSSW: CMSSW_14_0_X_2023-11-30-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43467/36241/install.sh to create a dev area with all the needed externals and cmssw changes.

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

@cmsbuild
Copy link
Contributor

Pull request #43467 was updated. @emeschi, @smorovic, @cmsbuild can you please check and sign again.

@smorovic
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c4eca/36527/summary.html
COMMIT: 9d13a12
CMSSW: CMSSW_14_0_X_2023-12-15-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43467/36527/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@smorovic
Copy link
Contributor

+daq

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 16cf9a6 into cms-sw:master Dec 19, 2023
11 checks passed
Copy link
Contributor

@makortel makortel left a comment

Choose a reason for hiding this comment

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

Given that the PR was already merged, I could also open an issue to follow up on these points (it was just easy to comment here). They should be addressed before I'd be comfortable for the RAW-level backward compatibility guarantee.

Copy link
Contributor

Choose a reason for hiding this comment

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

This test would still need a component where a ROOT file produced with a specific version of CMSSW (and stored in cms-data) is read with the current CMSSW.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What procedure should I follow to include the DataFormats-L1Scouting repository in cms-data? If that's unnecessary, where should I upload the test file?

Copy link
Contributor

Choose a reason for hiding this comment

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

The procedure is to ask @smuzaffar (or @cms-sw/externals-l2 ?) to create the DataFormats-L1Scouting and DataFormats-L1ScoutingRawData repositories to cms-data, and then make PRs to those repositories to add the test files. The in the CMSSW PR tests the cms-data PRs can be included so that all of them can be tested together.

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

New cms-data repos can be requested by created issue like #43621 . Once L2 sign the issue then bot will create the cms-data repo

Copy link
Contributor

Choose a reason for hiding this comment

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

by the way, I have created the issues for these two data repos. We need @cms-sw/daq-l2 to sign them in order to create these repos

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for creating the two repos, I've opened two PRs to add the test files. Once merged, I will add the reading procedure to the unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

This test would still need a component where a ROOT file produced with a specific version of CMSSW (and stored in cms-data) is read with the current CMSSW.

int hwIso_;
};

class Jet : public CaloObject {
Copy link
Contributor

Choose a reason for hiding this comment

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

If separate types are needed, please avoid the inheritance (to keep the structure simpler, and therefore easier to support for future evolution).

If separate types are not needed, how about just using CaloObject directly for the three cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that persistent data has already been created using these classes as they were in this PR (https://its.cern.ch/jira/browse/CMSTRANSF-799), perhaps it is easiest to just leave the inheritance as it is. The consequence is that the class hierarchy may not be changed in the future. In the PR adding the tests, please add comments here documenting this restriction.

@smuzaffar
Copy link
Contributor

We should have squashed/cleaned up this PR , it has added relatively large tmp.out file in the history now :-(

@smorovic
Copy link
Contributor

smorovic commented Dec 20, 2023

We should have squashed/cleaned up this PR , it has added relatively large tmp.out file in the history now :-(

Was the issue many actual commits, or that there was also one merge within them?
FWIW, I couldn't cleanly rebase that branch to CMSSW_13_X without a lot of conflicts (for running analysis on some data produced with same as this PR), I gave up and just applied a diff in the end.

@smuzaffar
Copy link
Contributor

too many commits or merge commit is not an issue. As bot reported in #43467 (comment), there were files which were added and removed which we should avoid (tmp.out was one so such file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants