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

Consolidate MTD modifiers #27450

Closed
kpedro88 opened this issue Jul 5, 2019 · 9 comments
Closed

Consolidate MTD modifiers #27450

kpedro88 opened this issue Jul 5, 2019 · 9 comments

Comments

@kpedro88
Copy link
Contributor

kpedro88 commented Jul 5, 2019

After #27449, the "tile" option for MTD is no longer exercised anywhere. I propose:

  • merging the modifiers phase2_timing_layer and phase2_timing_layer_bar
  • removing the modifier phase2_timing_layer_tile
  • simplifying the naming of Phase2 top-level Eras (since only Eras appended with "_timing_layer_bar" are used)
  • removing any extraneous code that supports the "tile" option
  • removing code only used for the old, TP implementation, e.g. FTLDigitizer and related classes (maybe even related data formats)

attn: @fabiocos @lgray @pmeridian @casarsa

@kpedro88
Copy link
Contributor Author

kpedro88 commented Jul 5, 2019

assign upgrade

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 5, 2019

New categories assigned: upgrade

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 5, 2019

A new Issue was created by @kpedro88 Kevin Pedro.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@fabiocos
Copy link
Contributor

fabiocos commented Jul 8, 2019

@kpedro88 once the tile scenario is removed what you propose makes sense. As it will stay available in older releases I think it should be ok, anyway I would like to understand from @pmeridian @lgray whether they envisage lates studies using that scenario (I doubt but...)

@casarsa
Copy link
Contributor

casarsa commented Jul 8, 2019

@kpedro88 @fabiocos @pmeridian @lgray To further clean and simplify the MTD code, could we also consider removing the old SimpleFTLDigitizer?

@kpedro88
Copy link
Contributor Author

kpedro88 commented Jul 8, 2019

@casarsa I highly encourage removing any code that is unused/unneeded after the workflow cleanup

@kpedro88
Copy link
Contributor Author

The first part of this issue is addressed in #28387

MTD developers should handle the actual code changes (removing unneeded features/classes/etc.)

@kpedro88
Copy link
Contributor Author

+upgrade
resolved by #28915

@cmsbuild
Copy link
Contributor

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
Projects
None yet
Development

No branches or pull requests

4 participants