HI updates for mixing in RelVal

[yetkinyilmaz]

The following updates are implemented:

-The vertex matching is fixed, and it's put into uniform format between standard vtx smearing modules, PbPb mixing and pPb mixing.
-The matrix is updated to have the correct options.
-Mixing-related modifications to standard sequences cleaned up.

Although independent, the pull request is needed before PR #1564 so that the needed RelVal operations can function properly. We need a RelVal production with this PR, and then another RelVal after PR #1564 is also merged. Details in:
https://twiki.cern.ch/twiki/pub/CMS/HiValidation53Xcontacts/03.02.14_DailyHIValidation.pdf

[cmsbuild]

A new Pull Request was created by @yetkinyilmaz for CMSSW_5_3_X.

HI updates for mixing in RelVal

It involves the following packages:

Configuration/Generator
Configuration/PyReleaseValidation
Configuration/StandardSequences
GeneratorInterface/HiGenCommon
SimGeneral/MixingModule

@vciulli, @civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @bendavid, @Degano, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan, @cbaus, @GiacomoSguazzoni, @rovere, @cerati 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.
@smuzaffar you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[yetkinyilmaz]

The HI validation and software contacts: @BetterWang @RylanC24 @kkrajczar @mandrenguyen
should also follow the progress.

On Feb 3, 2014, at 3:57 PM, cmsbuild notifications@github.com wrote:

A new Pull Request was created by @yetkinyilmaz for CMSSW_5_3_X.

HI updates for mixing in RelVal

It involves the following packages:

Configuration/Generator
Configuration/PyReleaseValidation
Configuration/StandardSequences
GeneratorInterface/HiGenCommon
SimGeneral/MixingModule

@vciulli, @civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @bendavid, @Degano, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @wmtan, @cbaus, @GiacomoSguazzoni, @rovere, @cerati 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.
@smuzaffar you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

—
Reply to this email directly or view it on GitHub.

[vlimant]

Hi Yetkin, the addition of yet another ad-hoc hi option is not appropriate IMO. This not going the in proper direction of integrating the HI configuration with ConfiBuilder. Most of it does. It's a -1 for me.
Switching digi cff can be done with -s DIGI:Configuration/StandardSequences/DigiHiMixSignal if the usage is not too widespread.

[yetkinyilmaz]

Hi Jean-Roch, it makes sense, let me try this.

On Feb 3, 2014, at 5:13 PM, vlimant notifications@github.com wrote:

Hi Yetkin, the addition of yet another ad-hoc hi option is not appropriate IMO. This not going the in proper direction of integrating the HI configuration with ConfiBuilder. Most of it does. It's a -1 for me.
Switching digi cff can be done with -s DIGI:Configuration/StandardSequences/DigiHiMixSignal if the usage is not too widespread.

—
Reply to this email directly or view it on GitHub.

[cmsbuild]

Pull request #2271 was updated. @vciulli, @civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @bendavid, @Degano, @davidlange6 can you please check and sign again.

[yetkinyilmaz]

The usage will be rare so the option is removed completely, and also we found it appropriate for this case to remove the additional cff as well, since this is rather the kind of thing to be done by customise.

On Feb 3, 2014, at 5:43 PM, cmsbuild notifications@github.com wrote:

Pull request #2271 was updated. @vciulli, @civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @bendavid, @Degano, @davidlange6 can you please check and sign again.

—
Reply to this email directly or view it on GitHub.

[vciulli]

+1

[civanch]

+1

[vlimant]

Looks better to me.

[davidlange6]

+1

On Feb 6, 2014, at 5:03 PM, vlimant notifications@github.com
wrote:

Looks better to me.

—
Reply to this email directly or view it on GitHub.

[franzoni]

Hello Yetkin, ... while I wait for a test job to finish a few comments;

• overall: how do these changes relate to the validation workflows which are in 44x ? Was the same 'vertex matching' problem present also back then ?
• Configuration/Generator/python/PyquenTuneZ2Settings_cff.py
  => does this change imply that the GEN-SIM we have now for background minBias events won't be mixable with the singnal GEN-SIM we'll get after this modification ?
• Configuration/PyReleaseValidation/python/relval_steps.py
  => in light of your change, the current step 'DIGIHISt3' is not used anywhere and is physically incorrect.
  If that was indeed the case, can you please remove it ?

[yetkinyilmaz]

Hi Giovanni;

-yes, the mixing workflow is also broken in 44x.

-no, the PyquenTuneZ2Settings are not used in the current workflows, but it will be needed for MC productions.

-yes, you are right, now I removed it in my local setup and it's fine. I can push the change, but will it cause us lose all the +1s of the day?

Thanks for the feedback.

On Feb 6, 2014, at 5:52 PM, franzoni notifications@github.com wrote:

Hello Yetkin, ... while I wait for a test job to finish a few comments;

overall: how do these changes relate to the validation workflows which are in 44x ? Was the same 'vertex matching' problem present also back then ?

Configuration/Generator/python/PyquenTuneZ2Settings_cff.py
=> does this change imply that the GEN-SIM we have now for background minBias events won't be mixable with the singnal GEN-SIM we'll get after this modification ?

Configuration/PyReleaseValidation/python/relval_steps.py
=> in light of your change, the current step 'DIGIHISt3' is not used anywhere and is physically incorrect.
If that was indeed the case, can you please remove it ?

—
Reply to this email directly or view it on GitHub.

[cmsbuild]

Pull request #2271 was updated. @vciulli, @civanch, @nclopezo, @vlimant, @mdhildreth, @cmsbuild, @franzoni, @bendavid, @Degano, @davidlange6 can you please check and sign again.

[franzoni]

Hello Yetkin,
thanks for your replies. I'd like to talk to you about "but it will be needed for MC productions" - in view of the production.

Question for you / Vladimir: both in step 3 and in step4 of workflow 300 there's one error message per event of this kind:

%MSG-w Product inconsistency: HiMixingModule:mix 06-Feb-2014 18:16:53 CET Run: 1 Event: 1
One of the sub-events is missing the product with type PCaloHit, instance HcalTB06BeamHits whereas the other one is fine.

In place of HcalTB06BeamHits, there can be also EcalTBH4BeamHits or CastorBU. All the steps complete w/o crash - yet, what kind of issue is this hinting ?

Cheers, Giovanni

[civanch]

Mike is the best person to consult. From my point of view HiMixingModule was not changed in this PR, so the problem exist independently, and from my point of view this warning requires extra code revision, because due the warning some actions are not performed in the code.

[yetkinyilmaz]

Yes, these warnings have always existed, since we have not implemented the mixing of these systems in the HiMixingModule (different than the MixingModule due to various reasons which I can explain of this topic).
We would like to of course improve this in the future but at the moment it is not in out immediate needs.

On Feb 6, 2014, at 7:56 PM, Vladimir Ivantchenko notifications@github.com wrote:

Mike is the best person to consult. From my point of view HiMixingModule was not changed in this PR, so the problem exist independently, and from my point of view this warning requires extra code revision, because due the warning some actions are not performed in the code.

—
Reply to this email directly or view it on GitHub.

[smuzaffar]

though the last commit
yetkinyilmaz@2e73fa0
reseted the signature but as commit is trivial and I have tested and all workflow ran fine, so I am including it

[smuzaffar]

+tested

[franzoni]

+1
for what concents matrix-related changes (20 events test with workflow 300 was ok).
HiMixingModule requires completing the development. And forward (71x) porting. Back porting is needed if we want to have the change of having a reference for validation.