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

remove L1Menu from postLS1 customization #10184

Conversation

mulhearn
Copy link
Contributor

This PR changes the postLS1 customizations to call only the common L1T post LS1 customizations, which do not include override of the L1Menu via XML file. Now the L1Menu should come from the GT, even when the postLS1 customization is called.

@mulhearn
Copy link
Contributor Author

@Martin-Grunewald is this what you have in mind?

@mulhearn
Copy link
Contributor Author

I've done some testing showing bitwise agreement in postLS1 workflows... more testing underway, but it looks OK to me.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mulhearn for CMSSW_7_6_X.

remove L1Menu from postLS1 customization

It involves the following packages:

SLHCUpgradeSimulations/Configuration

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@mulhearn
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.

@Martin-Grunewald
Copy link
Contributor

The HIon case needs to get the same treatment:

def customisePostLS1_HI(process):

@Martin-Grunewald
Copy link
Contributor

But be carefull as HIon does more than just load stage1 (as everyone else) and the HIon L1 menu.

@mulhearn
Copy link
Contributor Author

Yes, that's why I left it alone for now... but I can just remove the L1Menu customization part in the specific HI version. Doing that now...

@Martin-Grunewald
Copy link
Contributor

And can you please also make backport PRs for 75x AND 74x?
Thanks a lot!

@mulhearn
Copy link
Contributor Author

Yes, will do...

@mulhearn
Copy link
Contributor Author

Maybe the HIons L1Menus are not in the HIons GTs yet? It seems L1 still has PP menu when I run the HIons workflows. I think for now I'll leave this alone and sort it out separately, or does that ruin your plans somehow?

@Martin-Grunewald
Copy link
Contributor

Do you run the HIon wflows with an HIon GT?
On the GT side I only see the 74X PR #10151 (where HIon seems to be properly done), so I do not know the status of the 75X and 76X GTs. But from the HLT side our PRs now do the L1 menu
with the HLT menu so from that point of view we need just your the removal of the L1T setting
in PostLS1 (they are just not yet in the release, so jenkins tests on your side may fail).

@mulhearn
Copy link
Contributor Author

runTheMatrix.py -l 140.3 gives the following error when I remove the explicit L1Menu customization for HI:

----- Begin Fatal Exception 14-Jul-2015 10:31:36 CEST-----------------------
An exception of category 'FailModule' occurred while
[0] Processing run: 1 lumi: 1 event: 1
[1] Running path 'HLT_HIL1DoubleMu0_HighQ_v2'
[2] Calling event method for module HLTLevel1GTSeed/'hltL1sL1DoubleMuOpenBptxAND'
Exception Message:
Algorithm L1_DoubleMuOpen_BptxAND, requested as seed by a HLT path, not found in the L1 trigger menu
L1Menu_Collisions2015_25nsStage1_v3/L1T_Scales_20141121/Imp0
Incompatible L1 and HLT menus.
----- End Fatal Exception -------------------------------------------------

@mulhearn
Copy link
Contributor Author

Its 2 AM here in California, and I've got a 7 AM meeting... I have to go offline for a bit.

@Martin-Grunewald
Copy link
Contributor

@mulhearn

Could I move the HIon specific stuff BEYOND the common stage1 call, which is currently in
in from L1Trigger.L1TCommon.customsPostLS1 import customiseSimL1EmulatorForPostLS1_HI,
into SLHCUpgradeSimulations/Configuration/python/postLS1Customs.py directly?
That would preserve the parallel structure and we could clean up
L1Trigger.L1TCommon.customsPostLS1 from all those menu-specific customisation functions.
(BTW, I see that the HIon specific part is also different between 74X and 75X/76X... )

@mulhearn
Copy link
Contributor Author

Hmmm, I kind of hate to have parameters being set outside of L1Trigger. How about if we move those additional parts into HI into a separate hook and call those from the SLHC customizations? I'll check with HI folks about what we want to do about 74X/75X/76X differences.

@Martin-Grunewald
Copy link
Contributor

Fine with me of course!

@mulhearn
Copy link
Contributor Author

(BTW: I think all those 74X/75X/76X differences will finally be handled properly when CaloParams/CaloConfig goes into the GT...)

@mulhearn
Copy link
Contributor Author

(But handling L1Menu's first, as that seems most urgent!)

@mulhearn
Copy link
Contributor Author

Do you want me to just update my original PR with these changes, or do you want to do it yourself in the HLT PRs?

@mulhearn
Copy link
Contributor Author

Testing before updating PR...

@Martin-Grunewald
Copy link
Contributor

I absorb things in my PRs as I need HLT separate for all 74/75/76 - but of course need to know what to change :)

@mulhearn
Copy link
Contributor Author

Yeah, its coming shortly... tests on HI worked fine.

@mulhearn
Copy link
Contributor Author

I think now we are OK on configuration but still need to sort out the problem with GTs....

@cmsbuild
Copy link
Contributor

Pull request #10184 was updated. @cmsbuild, @civanch, @mulhearn, @mdhildreth can you please check and sign again.

# HI L1Menu:
from L1Trigger.Configuration.customise_overwriteL1Menu import L1Menu_CollisionsHeavyIons2015_v0
process = L1Menu_CollisionsHeavyIons2015_v0(process)

Copy link
Contributor

Choose a reason for hiding this comment

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

Here the L1 menu is still loaded... presumably because in 76X it is not yet in the GT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly.

@mulhearn
Copy link
Contributor Author

I found the problem with the GT pull request... not my fault!

@Martin-Grunewald
Copy link
Contributor

@mulhearn
Please have a look at the big HLT PRs #9878 (74X), #9684 (75X), #10209 (76X),
where I took your L1 manipulations and adapted them to the other CMSSW releases,
taking into account that some modified L1 files were different between releases.
I have also removed the HIon loading, of course. TSG tests run fine but for jenkins
tests the GTs must become available.

@davidlange6 davidlange6 merged commit dc7ae4b into cms-sw:CMSSW_7_6_X Jul 24, 2015
@mulhearn mulhearn deleted the l1t-remove-l1menu-from-customs-76x branch October 18, 2015 21:20
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

4 participants