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

Fast shower01 #2886

Merged
merged 2 commits into from
Mar 17, 2014
Merged

Fast shower01 #2886

merged 2 commits into from
Mar 17, 2014

Conversation

Vlandr57
Copy link
Contributor

Preparation for Shashlik shower in ECAL for FastSim

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Vlandr57 for CMSSW_6_2_X_SLHC.

Fast shower01

It involves the following packages:

FastSimulation/CaloHitMakers
FastSimulation/ShowerDevelopment

@cmsbuild, @Degano, @giamman, @lveldere, @nclopezo can you please review it and eventually sign? Thanks.
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.
@andersonjacob, @mark-grimes you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@@ -387,7 +387,7 @@
else
{
double x0=segiterator->x0FromCm(dist);
if(x0<maxX0_) maxX0_=x0;
if(x0>maxX0_) maxX0_=x0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, can you please comment on this change? Was the previous behaviour wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was introduced by Maxime Gouzevitch and his comment below:

This is a feature. Those lines was designed to decide if a detailed shower tail
shall be used or not. It is a question of speed vs details. It looks like in
version before a detailed shower tail was always called and the condiotion line 390
was useless. Now it allow to decide if one need or not detailed shower tail.

Florian may explain better the point why those lines have to be changed:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.
Andrea


Andrea Giammanco
Office phone: +32 10 478185
Mobiles: +32 493 581662 (BE), +41 76 2323672 (CH), +39 349 5552471 (IT), +972 546307196 (IL)


From: Vlandr57 [notifications@github.com]
Sent: 17 March 2014 12:37
To: cms-sw/cmssw
Cc: Andrea Giammanco
Subject: Re: [cmssw] Fast shower01 (#2886)

In FastSimulation/CaloHitMakers/src/EcalHitMaker.cc:

@@ -387,7 +387,7 @@
else
{
double x0=segiterator->x0FromCm(dist);

  •               if(x0<maxX0_) maxX0_=x0;
    
  •               if(x0>maxX0_) maxX0_=x0;
    

This change was introduced by Maxime Gouzevitch and his comment below:

This is a feature. Those lines was designed to decide if a detailed shower tail
shall be used or not. It is a question of speed vs details. It looks like in
version before a detailed shower tail was always called and the condiotion line 390
was useless. Now it allow to decide if one need or not detailed shower tail.

Florian may explain better the point why those lines have to be changed:


Reply to this email directly or view it on GitHubhttps://github.com//pull/2886/files#r10652815.

@Vlandr57
Copy link
Contributor Author

+1

On Mon, 17 Mar 2014, cmsbuild wrote:

A new Pull Request was created by @Vlandr57 for CMSSW_6_2_X_SLHC.

Fast shower01

It involves the following packages:

FastSimulation/CaloHitMakers
FastSimulation/ShowerDevelopment

@cmsbuild, @Degano, @giamman, @lveldere, @nclopezo can you please review it and
eventually sign? Thanks.
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.
@andersonjacob, @mark-grimes 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 onGitHub.[5694963__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcxMDY3NDAwMywiZGF0
YSI6eyJpZCI6Mjc4MzU0MDh9fQ==--5bc4cb1c7789a67cb44e90f3ce211e1becf3b314.gif]

@andersonjacob
Copy link
Contributor

merge

tests run with only known failures

cmsbuild added a commit that referenced this pull request Mar 17, 2014
@cmsbuild cmsbuild merged commit b49495e into cms-sw:CMSSW_6_2_X_SLHC Mar 17, 2014
@lveldere
Copy link
Contributor

+1

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

5 participants