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

change the raw format for 48-chan feds, back to the old one #16164

Merged
merged 1 commit into from Oct 13, 2016

Conversation

dkotlins
Copy link
Contributor

Change back phase1 raw-fed format to what is used for phase0.
With 48-channel FEDs we do not need the new format.
Needs the new (post 8_1_0_pre12) phase1 MC GTs.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dkotlins for CMSSW_8_1_X.

It involves the following packages:

EventFilter/SiPixelRawToDigi

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @VinInn this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Oct 10, 2016

@dkotlins
does this change mean that we can not read phase-1 pre12 files?

@slava77
Copy link
Contributor

slava77 commented Oct 10, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 10, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15626/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@dkotlins
Copy link
Contributor Author

Slava,
It is like before, each time the cabling map changes the raw data become incompatible.
Of course we can always rerun digi2raw and everything will be fine.
Has there been any MC production with _pre12.
Best regards,
Danek

On 10 Oct 2016, at 18:11, Slava Krutelyov notifications@github.com wrote:

@dkotlins
does this change mean that we can not read phase-1 pre12 files?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@slava77
Copy link
Contributor

slava77 commented Oct 10, 2016

On 10/10/16 12:01 PM, dkotlins wrote:

Slava,
It is like before, each time the cabling map changes the raw data become
incompatible.
Of course we can always rerun digi2raw and everything will be fine.
Has there been any MC production with _pre12.

Hi Danek,

we have pre12 relval samples.

This particular change doesn't look like a typical cabling map change.
Typically the map is an condition payload (even a text file) which can
be swapped out
to read corresponding data appropriately.
This change requires code change and recompilation.
If the constants like the one you changed can possibly change as a part
of detector configuration,
maybe it's worth to put these in conditions.

    --slava

Best regards,
Danek

On 10 Oct 2016, at 18:11, Slava Krutelyov notifications@github.com wrote:

@dkotlins
does this change mean that we can not read phase-1 pre12 files?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16164 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcboyb_uoaY2y4hfBXg9nWHwSCjgl7ks5qyouBgaJpZM4KSrCD.

@dkotlins
Copy link
Contributor Author

Slava,
Yes, you are right it is not just a cabling map change but it goes together with the map change.
Recently we decided to have 48 channel FEDs (implemented in new v2 map) instead of the planned
96 channel (the old v1 map).
This change just affects the GT. But because of 48 channel/FED we do not need a data format change like
the 96channel version, hence I am reverting the format to the old one (less bits reserve for the channel coding).
So, the 2 changes have to go together. AlcaDB already has a GT with the new cabling.
BEst regards,
Danek

On 10 Oct 2016, at 21:22, Slava Krutelyov notifications@github.com wrote:

On 10/10/16 12:01 PM, dkotlins wrote:

Slava,
It is like before, each time the cabling map changes the raw data become
incompatible.
Of course we can always rerun digi2raw and everything will be fine.
Has there been any MC production with _pre12.

Hi Danek,

we have pre12 relval samples.

This particular change doesn't look like a typical cabling map change.
Typically the map is an condition payload (even a text file) which can
be swapped out
to read corresponding data appropriately.
This change requires code change and recompilation.
If the constants like the one you changed can possibly change as a part
of detector configuration,
maybe it's worth to put these in conditions.

--slava

Best regards,
Danek

On 10 Oct 2016, at 18:11, Slava Krutelyov notifications@github.com wrote:

@dkotlins
does this change mean that we can not read phase-1 pre12 files?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16164 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcboyb_uoaY2y4hfBXg9nWHwSCjgl7ks5qyouBgaJpZM4KSrCD.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@mmusich
Copy link
Contributor

mmusich commented Oct 11, 2016

@dkotlins @slava77
the needed change in conditions is already implemented and merged in
#16101

@cvuosalo
Copy link
Contributor

+1

For #16164 381563b

Changing 2017 Pixel RAW format to support only 48-channel FEDs instead of 96 channels that won't be needed. There should be no change in monitored quantities.

The code change is satisfactory, and Jenkins tests against baseline CMSSW_8_1_X_2016-10-10-1100 show no significant differences, as expected.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 8657cbb into cms-sw:CMSSW_8_1_X Oct 13, 2016
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

6 participants