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.

Sign up for GitHub

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

Jump to bottom

Update L1 tracking for HL-LHC (fixed) #34028

Closed
wants to merge 466 commits into from
Closed

Update L1 tracking for HL-LHC (fixed) #34028

wants to merge 466 commits into from

Conversation

skinnari
Copy link
Contributor

@skinnari skinnari commented Jun 8, 2021

PR description:

This PR is a fixed version of #33837 , made to master branch instead of CMSSW_11_3_X plus fixing "tests failed" (that were due to changes around handling of data files).

This PR includes updates to the L1 tracking for HL-LHC:

  • use DTC emulation (L1Trigger/TrackerDTC), the step between the Tracker and the L1 tracking system, along with associated, necessary changes to the L1 tracking code
  • algorithm updates to match HLS-based version (i.e. firmware)
  • code cleanup (removal of obsolete code, general code improvements)

Note:

PR validation:

Validated via L1 tracking performance studies on ttbar PU=200 samples.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This is not a backport.

@cmsbuild cmsbuild added this to the CMSSW_12_0_X milestone Jun 8, 2021
This was referenced Jun 8, 2021
Closed
Merged
@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34028/23158

  • This PR adds an extra 992KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34028/23161

  • This PR adds an extra 800KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34028/23162

  • This PR adds an extra 800KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 8, 2021

A new Pull Request was created by @skinnari (Louise Skinnari) for master.

It involves the following packages:

L1Trigger/TrackFindingTMTT
L1Trigger/TrackFindingTracklet
L1Trigger/TrackerDTC

@cmsbuild, @rekovic, @srimanob, @cecilecaillol, @kpedro88 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @erikbutz this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cecilecaillol
Copy link
Contributor

please test

@skinnari
Copy link
Contributor Author

ugh I am sorry, I had to rebase it since I had used a nightly when setting up that was no longer available, and it seems that it now committed that whole history. do i need to close and reopen again?

@skinnari
Copy link
Contributor Author

I think I have to close this and reopen again. I am very sorry. I messed up git and not sure how to undo. For the future, is there a way to directly know which release one should use when making PRs? I had initially used a CMSSW_11_3_X version but had to make PR to master, and then picked the most recent nightly (which now was no longer available and I had to move to a different release)...

@skinnari skinnari closed this Jun 22, 2021
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34028/23453

@cmsbuild
Copy link
Contributor

Pull request #34028 was updated. @SiewYan, @andrius-k, @gouskos, @lveldere, @chayanit, @wajidalikhan, @sbein, @ianna, @kpedro88, @Martin-Grunewald, @rekovic, @tlampen, @alberto-sanchez, @pohsun, @santocch, @cecilecaillol, @perrotta, @civanch, @yuanchao, @makortel, @ErnestaP, @ahmad3213, @cmsbuild, @agrohsje, @fwyzard, @GurpreetSinghChahal, @smorovic, @davidlange6, @smuzaffar, @Dr15Jones, @jpata, @cvuosalo, @mdhildreth, @jfernan2, @slava77, @ggovi, @qliphy, @fabiocos, @francescobrivio, @malbouis, @ssekmen, @mkirsano, @jordan-martins, @kmaeshima, @emeschi, @alja, @srimanob, @silviodonato, @fgolf, @mariadalfonso, @rvenditti can you please check and sign again.

@kpedro88
Copy link
Contributor

@skinnari "rebase" is a word with a precise meaning. it does not mean "merge". in the future, please follow http://cms-sw.github.io/tutorial-resolve-conflicts.html to rebase correctly.

@kpedro88
Copy link
Contributor

to undo merges (or other mistakes) in git, there are several options:

  1. try http://cms-sw.github.io/tutorial-resolve-conflicts.html#recovering-from-a-bad-rebase-or-merge
  2. go back to your old working area in the previous release and push that state of your branch
  3. use git reflog

@skinnari
Copy link
Contributor Author

thanks @kpedro88, I think I have fixed it now, but because the instructions contains a "git push -f " it seems i cannot reopen the PR now?

@kpedro88
Copy link
Contributor

Sometimes this happens when push -f is done after a PR is closed; I'm not sure why. I advise that, in the future, don't close PRs prematurely. (You may be able to push -f from the broken/merged branch if you still have it, reopen the PR, and then push -f from the non-merged branch again; but I leave it up to you whether you want to try this or just make a new PR.)

@skinnari skinnari mentioned this pull request Jun 22, 2021
Merged
@makortel
Copy link
Contributor

Sometimes this happens when push -f is done after a PR is closed; I'm not sure why.

Some explanation in isaacs/github#361

We're blocking the pull request reopen if the current head isn't a descendant of the stored head sha (which is what the head was when the pull request was closed). We are not allowing the reopen in that case, because there is no good way to tell what changes have happened while a pull request was closed and the head branch has changed.

cmsbuild added a commit that referenced this pull request Jul 7, 2021
@cmsbuild
Merge pull request #34215 from cms-L1TK/L1TK-PR-june2021
5920cbe 
Update L1 tracking for HL-LHC (replacement of #34028)
This was referenced Jul 7, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aryd aryd aryd left review comments

@srimanob srimanob srimanob left review comments

Assignees
No one assigned
Labels
alca-pending analysis-pending code-checks-approved core-pending daq-pending db-pending dqm-pending fastsim-pending generators-pending geometry-pending heterogeneous-pending hlt-pending l1-pending operations-pending orp-pending pdmv-pending pending-signatures reconstruction-pending simulation-pending tests-pending upgrade-pending visualization-pending xpog-pending
Projects
None yet
Milestone
CMSSW_12_0_X
Development

Successfully merging this pull request may close these issues.