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

Clean up obsolete Phase2 geometries in 11_3 #33064

Closed
srimanob opened this issue Mar 4, 2021 · 24 comments
Closed

Clean up obsolete Phase2 geometries in 11_3 #33064

srimanob opened this issue Mar 4, 2021 · 24 comments
Labels
fully-signed upgrade-approved

Comments

@srimanob
Copy link
Contributor

srimanob commented Mar 4, 2021 •
edited

This is a proposal (to be implemented in _pre5) to clean up obsolete Phase2 geometries in 11_3. Please comment if you would like to keep additional versions.

  • D49 = T15+C9+M4+I10+O4+F2 (Keep, HLT TDR)
  • D50 = T15+C9+M4+I11+O4+F2 (Delete)
  • D60 = T15+C10+M4+I10+O4+F3 (Keep, HGCal with HFNose)
  • D64 = T22+C11+M4+I11+O5+F4 (Delete)
  • D65 = T23+C11+M4+I11+O5+F4 (Delete)
  • D66 = T21+C11+M8+I11+O5+F4 (Delete)
  • D67 = T21+C11+M9+I11+O5+F4 (Delete)
  • D68 = T21+C11+M6+I11+O5+F4 (Delete)
  • D69 = T21+C12+M6+I11+O5+F5 (Delete)
  • D70 = T21+C13+M7+I11+O6+F6 (Delete)
  • D71 = T21+C14+M7+I11+O7+F6 (Delete)
  • D72 = T21+C11+M6+I12+O5+F4 (Keep, TDR ETL)
  • D74 = T21+C14+M9+I11+O7+F6 (Delete)
  • D75 = T21+C14+M7+I13+O7+F6 (Delete)
  • D76 = T21+C14+M9+I13+O7+F6 (Keep)
  • D77 = T24+C14+M9+I13+O7+F6 (Keep)
  • D78 = T22+C14+M9+I13+O7+F6 (Keep)
  • D79 = T23+C14+M9+I13+O7+F6 (Keep)

FYI @mmusich @fabiocos @bsunanda @civanch

I don't know how to make a list of non-used XML geometry files. Do we have tools or a way to check?
@davidlange6 @silviodonato @qliphy
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2021

A new Issue was created by @srimanob Phat Srimanobhas.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Mar 4, 2021

@emiglior FYI

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2021

New categories assigned: upgrade

@kpedro88,@srimanob you have been requested to review this Pull request/Issue and eventually sign? Thanks

@srimanob
Copy link
Contributor Author

srimanob commented Mar 4, 2021

assign upgrade

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 4, 2021

New categories assigned: upgrade

@kpedro88,@srimanob you have been requested to review this Pull request/Issue and eventually sign? Thanks

@slava77
Copy link
Contributor

slava77 commented Mar 11, 2021

What are the long-term plans to keep

  • D49 = T15+C9+M4+I10+O4+F2 (Keep, HLT TDR)
  • D60 = T15+C10+M4+I10+O4+F3 (Keep, HGCal with HFNose)
  • D72 = T21+C11+M6+I12+O5+F4 (Keep, TDR ETL)
    ?

My question is in the context of having to support GE0 software for pre-M9 muon system.
@watson-ij @jshlee please clarify if the migration from ME0 algorithms to GE0 implies that the GE0 algorithms can still support ME0 geometries or does it mean that we'd have to keep the ME0 specific algorithms alive for this.

@jshlee
Copy link
Contributor

jshlee commented Mar 11, 2021

@slava77 - GE0 algorithms do not support ME0 geometries.

@slava77
Copy link
Contributor

slava77 commented Mar 11, 2021

@slava77 - GE0 algorithms do not support ME0 geometries.

thank you for clarifying.
Let's try to understand how long the geometries proposed to be kept/supported (D49, D60, and D72) are expected to be maintained.
@srimanob

@kpedro88
Copy link
Contributor

Historically we have kept TDR baseline geometries around even significantly after the TDR is published (i.e. ≥1 year). This is because the TDR review committees ask questions like "what if you use [new reco algorithm] on [old TDR geometry]?" or "what is the impact of [proposed geometry change]?", so having the old geometries in old releases is sometimes not sufficient.

@jshlee
Copy link
Contributor

jshlee commented Mar 11, 2021

@kpedro88 - the geometry isn't different between ME0 and GE0 so this shouldn't be an issue. Currently, the digitiser is the main difference and then the ME0 rechit producer. In both cases, GEM/GE0 is implemented more correctly and realistically. I'm happy to have ME0 geometry and code kept for a while, but we don't want to have samples produce with them anymore.

@fabiocos
Copy link
Contributor

A comment about D72: in principle we could even remove this scenario, and possibly add back a new one with MTD I12 inserted in a more modern combination, in case it is requested for comparisons. I12 in D72 corresponds anyway to a design that looks abandoned, so keeping the xml description in the release should be enough

@srimanob
Copy link
Contributor Author

A comment about D72: in principle we could even remove this scenario, and possibly add back a new one with MTD I12 inserted in a more modern combination, in case it is requested for comparisons. I12 in D72 corresponds anyway to a design that looks abandoned, so keeping the xml description in the release should be enough

Hi @fabiocos
Is that simple to use I12 in D76? If not, maybe keep D72 for now could be an option. Thanks.

@fabiocos
Copy link
Contributor

@srimanob I have discussed internally in MTD, and we agree that the I12 option is not something we need to routinely probe, or even plan to use anymore. Since we have the xml description, which in case could be cross checked against recent envelopes and adapted to it, we may keep it, but no real need to have an I12-based scenario in the release. That detector design looks obsolete by now, and it is unlikely we will want to go back to comparisons.

So I think it is advisable and simpler to just remove D72, in case we will provide a newer scenario with that ETL geometry if needed at all.

@emiglior
Copy link
Contributor

As discussed in the last meeting of the Inner Tracker sensors performance task force,
https://indico.cern.ch/event/988838/contributions/4258167/attachments/2202703/3736635/em20210305.pdf
we need to explore few additional layouts for the ph2 Tracker corresponding to more realistic construction scenarios (e.g. limited surface that can be instrumented with 3D sensors)

In summary, we propose

  • T21, T22 (Keep)
  • T23 (Delete)
  • T25, T26 (New)

As in the past, we need the new geometries in CMSSW for validating them with PU200 RelVal.
Clearly we do not want to interfere with this issue, but it would be great if the changes we propose can still enter in pre5.

@adewit FYI

@srimanob
Copy link
Contributor Author

@emiglior @mmusich
Thanks for the comment? How quick can you put the new geometry in?
As discussed in the release meeting last week, we may build pre5 earlier than the schedule, to open a chance for longer DD4Hep validation.

FYI @silviodonato @qliphy

@mmusich
Copy link
Contributor

mmusich commented Mar 18, 2021

Thanks for the reply @srimanob.
@adewit will do the PR to integrate the new geometries (partially internally validated) by tomorrow morning (CERN time).

@silviodonato
Copy link
Contributor

@emiglior @mmusich
Thanks for the comment? How quick can you put the new geometry in?
As discussed in the release meeting last week, we may build pre5 earlier than the schedule, to open a chance for longer DD4Hep validation.

FYI @silviodonato @qliphy

I haven't received any news from DD4HEP yet, so I think we will build pre5 next week.

@adewit adewit mentioned this issue Mar 19, 2021
Merged
@mmusich
Copy link
Contributor

mmusich commented Mar 19, 2021

Hi @silviodonato the PR mentioned is here #33222, do I understand correctly that in the meanwhile the DD4HEP issues have been straightened out and pre5 is being built this weekend?
Waiting 4+ weeks to have these geometries in to submit relvals is going to be a substantial setback for the Tracker project.
Thanks for the clarification.

@bsunanda
Copy link
Contributor

bsunanda commented Mar 19, 2021 via email

@silviodonato
Copy link
Contributor

Hi @mmusich , yes, DD4HEP is ready for a validation so the idea is to build pre5 during the weekend.
CMSSW_11_3_0_pre6 is scheduled for April 6, so in two weeks and half from now ( https://twiki.cern.ch/twiki/bin/view/CMS/CMSSW_11_3_0 ).
If you think #33222 is ready and can be merged soon (~Monday), we can wait for it to build pre5

@srimanob
Copy link
Contributor Author

Update the proposal:

Keep

  • D49 = T15+C9+M4+I10+O4+F2 (Keep, HLT TDR)
  • D60 = T15+C10+M4+I10+O4+F3 (HFNose)
  • D76 = T21+C14+M9+I13+O7+F6
  • D77 = T24+C14+M9+I13+O7+F6
  • D78 = T22+C14+M9+I13+O7+F6
  • D80 = T25+C14+M9+I13+O7+F6
  • D81 = T26+C14+M9+I13+O7+F6

Delete

  • D50 = T15+C9+M4+I11+O4+F2
  • D64 = T22+C11+M4+I11+O5+F4
  • D65 = T23+C11+M4+I11+O5+F4
  • D66 = T21+C11+M8+I11+O5+F4
  • D67 = T21+C11+M9+I11+O5+F4
  • D68 = T21+C11+M6+I11+O5+F4
  • D69 = T21+C12+M6+I11+O5+F5
  • D70 = T21+C13+M7+I11+O6+F6
  • D71 = T21+C14+M7+I11+O7+F6
  • D72 = T21+C11+M6+I12+O5+F4
  • D74 = T21+C14+M9+I11+O7+F6
  • D75 = T21+C14+M7+I13+O7+F6
  • D79 = T23+C14+M9+I13+O7+F6

@srimanob srimanob mentioned this issue Mar 30, 2021
Merged
@bsunanda
Copy link
Contributor

bsunanda commented Mar 30, 2021 via email

@srimanob
Copy link
Contributor Author

srimanob commented Apr 1, 2021

+Upgrade

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 1, 2021

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fully-signed upgrade-approved
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
10 participants
@fabiocos @cmsbuild @kpedro88 @slava77 @srimanob @bsunanda @mmusich @emiglior @jshlee @silviodonato