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

Fix position of unconstrained pT coming from OMTF #44798

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

dinyar
Copy link
Contributor

@dinyar dinyar commented Apr 22, 2024

PR description:

Due to a miscommunication the incoming unconstrained pT from OMTF as expected at the wrong position in the bitword on the link. This PR fixes this. (A backport will be needed to fix DQM mismatches.)

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 22, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44798/40035

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dinyar for master.

It involves the following packages:

  • L1Trigger/L1TMuon (l1)

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

cms-bot commands are listed here

@aloeliger
Copy link
Contributor

type bug-fix

@aloeliger
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: Build ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e8448b/38992/summary.html
COMMIT: f41ee91
CMSSW: CMSSW_14_1_X_2024-04-22-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44798/38992/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling  src/L1Trigger/L1TMuon/src/MuonTriggerPrimitive.cc
>> Compiling  src/L1Trigger/L1TMuon/src/RegionalMuonRawDigiTranslator.cc
>> Compiling  src/L1Trigger/L1TMuon/src/SubsystemCollector.cc
>> Compiling  src/L1Trigger/L1TMuon/src/SubsystemCollectorFactory.cc
src/L1Trigger/L1TMuon/src/RegionalMuonRawDigiTranslator.cc: In static member function 'static void l1t::RegionalMuonRawDigiTranslator::generatePackedDataWords(const l1t::RegionalMuonCand&, uint32_t&, uint32_t&, bool, bool, bool)':
src/L1Trigger/L1TMuon/src/RegionalMuonRawDigiTranslator.cc:191:41: error: no matching function for call to 'l1t::RegionalMuonRawDigiTranslator::generateRawTrkAddress(const l1t::RegionalMuonCand&, const bool&, const bool&)'
  191 |   int rawTrkAddr = generateRawTrkAddress(mu, isKbmtf, useOmtfDisplacementInfo);
      |                    ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from src/L1Trigger/L1TMuon/src/RegionalMuonRawDigiTranslator.cc:2:
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_1_X_2024-04-22-1100/src/L1Trigger/L1TMuon/interface/RegionalMuonRawDigiTranslator.h:45:16: note: candidate: 'static int l1t::RegionalMuonRawDigiTranslator::generateRawTrkAddress(const l1t::RegionalMuonCand&, bool)'
   45 |     static int generateRawTrkAddress(const RegionalMuonCand&, bool isKalman);


Clang Build

I found compilation error 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'

>> Local Products Rules ..... started
>> Local Products Rules ..... done
>> Creating project symlinks
>> Entering Package L1Trigger/L1TMuon
>> Compile sequence completed for CMSSW CMSSW_14_1_X_2024-04-22-1100
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 1
+ eval scram build outputlog '&&' '(python3' /data/cmsbld/jenkins/workspace/ib-run-pr-tests/cms-bot/buildLogAnalyzer.py --logDir /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_14_1_X_2024-04-22-1100/tmp/el8_amd64_gcc12/cache/log/src '||' 'true)'
++ scram build outputlog
>> Entering Package L1Trigger/L1TMuon
Entering library rule at src/L1Trigger/L1TMuon/plugins
>> Compiling edm plugin src/L1Trigger/L1TMuon/plugins/L1TBMTFConverter.cc


@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44798/40038

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

Pull request #44798 was updated. @epalencia, @aloeliger, @cmsbuild can you please check and sign again.

@epalencia
Copy link
Contributor

Please test

@dinyar dinyar marked this pull request as draft April 22, 2024 14:39
@missirol
Copy link
Contributor

Hi @dinyar

Just a naive question, for my own understanding.

Is it correct that this PR is relevant to online-dqm, but it does not affect the unpacking of L1T data that we do online at HLT [1] (so, the online HLT is blind to this specific PR) ?

Thanks
Marino

[1]

process.hltGtStage2Digis = cms.EDProducer( "L1TRawToDigi",
    FedIds = cms.vint32( 1404 ),
    Setup = cms.string( "stage2::GTSetup" ),
    FWId = cms.uint32( 0 ),
    DmxFWId = cms.uint32( 0 ),
    FWOverride = cms.bool( False ),
    TMTCheck = cms.bool( True ),
    CTP7 = cms.untracked.bool( False ),
    MTF7 = cms.untracked.bool( False ),
    InputLabel = cms.InputTag( "rawDataCollector" ),
    lenSlinkHeader = cms.untracked.int32( 8 ),
    lenSlinkTrailer = cms.untracked.int32( 8 ),
    lenAMCHeader = cms.untracked.int32( 8 ),
    lenAMCTrailer = cms.untracked.int32( 0 ),
    lenAMC13Header = cms.untracked.int32( 8 ),
    lenAMC13Trailer = cms.untracked.int32( 8 ),
    debug = cms.untracked.bool( False ),
    MinFeds = cms.uint32( 0 )
)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e8448b/39000/summary.html
COMMIT: 2437873
CMSSW: CMSSW_14_1_X_2024-04-22-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44798/39000/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@dinyar
Copy link
Contributor Author

dinyar commented Apr 23, 2024

Hi @dinyar

Just a naive question, for my own understanding.

Is it correct that this PR is relevant to online-dqm, but it does not affect the unpacking of L1T data that we do online at HLT [1] (so, the online HLT is blind to this specific PR) ?

Thanks Marino

hi @missirol,

yes, exactly. this should only affect DQM but not HLT.

Cheers,
Dinyar

@dinyar dinyar marked this pull request as ready for review April 23, 2024 14:13
@dinyar
Copy link
Contributor Author

dinyar commented Apr 23, 2024

Briefly discussed with @kbunkow whether the emulator should re-emulate the faulty behaviour for those data that were taken like that, the outcome of it was that it shouldn't (as is currently done in the PR), so this is ready to merge.

@eyigitba
Copy link
Contributor

Hi Dinyar, since we didn't collect any data with the displaced OMTF algorithm I also think that it should be fine like this. Thanks

@aloeliger
Copy link
Contributor

As far as I am concerned, this change is trivial and can go in immediately. @dinyar @eyigitba Any final expected changes?

@dinyar
Copy link
Contributor Author

dinyar commented Apr 24, 2024

No, from my side this is ready to go. Thanks!

@aloeliger
Copy link
Contributor

+l1

@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 a414e7d into cms-sw:master Apr 24, 2024
11 checks passed
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.

None yet

7 participants