search

cms-l1t-offline / cmssw

CMS Offline Software
cms-sw.github.io/cmssw
Apache License 2.0
17 stars 27 forks source link

Revert NN tau variable mapping #1238

Closed HaarigerHarald closed 7 months ago

HaarigerHarald commented 7 months ago

Fixes #1237

artlbv commented 7 months ago

I tested it and it worked fine.

Please merge this ASAP as others can't use the IB.. @aloeliger @epalencia

triggerDoctor commented 7 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

Attempts to compile this PR succeeded!

Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -j 8
triggerDoctor commented 7 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found no issues with the code checks! Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 code-checks && scram b -k -j 8 code-checks
I found no issues with the headers! Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 check-headers
triggerDoctor commented 7 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

I found 1 files that did not meet formatting requirements:

Please run scram b code-format to auto-apply code formatting Info Value
return code 0
command evalscramv1 runtime -sh&& scram b -k -j 8 code-format-all
triggerDoctor commented 7 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation.

This PR passes available unit tests! Info Value
return code 0
command evalscramv1 runtime -sh&& scram b runtests
triggerDoctor commented 7 months ago

Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation

I found a non-zero return code running the relval workflows for this PR! Info Value
return code 1
command evalscramv1 runtime -sh&& runTheMatrix.py --what upgrade -l 26834.78
aloeliger commented 7 months ago

Hmm. The script output is wrong, but the return code is wrong. Let me check.

aloeliger commented 7 months ago

HLT overlap. Not this PR's issue.

aloeliger commented 7 months ago

@artlbv I am merging this PR.

@HaarigerHarald Could you please open a PR for this to CMSSW as soon as practical?

artlbv commented 7 months ago

thanks @aloeliger

on

@HaarigerHarald Could you please open a PR for this to CMSSW as soon as practical?

this should be really included in the same PR as https://github.com/cms-l1t-offline/cmssw/pull/1225#issuecomment-2031755976 so more for @duchstf

Duchstf commented 7 months ago

@artlbv Hello, since it was #1225 was merged, how should I proceed? do I need to create a PR?

artlbv commented 7 months ago

@Duchstf actually you probably should not do anything.. the P2GT emulator has not been updated in central yet. the PR is on hold: https://github.com/cms-l1t-offline/cmssw/pull/1207

aloeliger commented 7 months ago

I really don't want to lose either of these PRs to forgetfulness because we were all so busy with other phase 2 things. We should either:

  1. Revert this and #1225 and immediately reopen a revert-revert to remind us that this is going to go in
  2. Merge this and #1225 in a mini-update to CMSSW immediately.

This fork is only a convenience. Getting it in CMSSW is what actually matters.

artlbv commented 7 months ago
  1. is the way to go..
  2. will break the current P2GT emulator as it parses the Tau "isolation" for the ID: https://github.com/cms-sw/cmssw/blob/CMSSW_14_0_0_pre3/L1Trigger/Phase2L1GT/python/l1tGTMenu_lepSeeds_cff.py#L395 and fixing this will require the above P2GT PR in CMSSW
HaarigerHarald commented 7 months ago

Why not open a PR to central that basically includes both #1225 and #1238 ?

aloeliger commented 7 months ago

@HaarigerHarald @Duchstf Ping again on opening a PR to CMSSW combining #1225 and #1238. Please do not lose these changes.

HaarigerHarald commented 7 months ago

@Duchstf pls do it as it mostly concerns stuff in the correlator package. You can cherry-pick my commit into your branch and then just open a PR from it.

Edit: I opened a PR now