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

adding a layered calorimeter struct that copes with conical calorimeters #132

Merged
merged 4 commits into from
Mar 24, 2017

Conversation

Voutsi
Copy link
Contributor

@Voutsi Voutsi commented Mar 23, 2017

BEGINRELEASENOTES

  • modifying the LayeredCalorimeterData struct in order to cope with conical shaped calorimeters

ENDRELEASENOTES

Copy link

@petricm petricm left a comment

Choose a reason for hiding this comment

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

As far as I see this is identical to the not conical with the difference that double extent[6] ; instead of double extent[4] ;. Wouldn't it then be better to modify LayeredCalorimeterStruct and put an if in the overloading checking if the two last fields are filled?



/** Azimuthal angle of the first module in barrel layout
* DEPRECATED! PLEASE POPULATE INNER/OUTER PHI0 INSTEAD
Copy link

Choose a reason for hiding this comment

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

Why are you even putting it in if it is deprecated?

io << " Layers : " << std::endl
<< " distance inner_nX0 outer_nX0 inner_nInt outer_nInt inner_thick outer_thick sense_thick"
<< std::endl ;
//"distance inner_nX0 outer_nX0 inner_nLambda outer_nLambda inner_thick outer_thick sensitive_thick" << std::endl ;
Copy link

Choose a reason for hiding this comment

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

What's the purpose of this comment?

@@ -195,6 +239,6 @@ namespace DD4hep {
return io ;
}


Copy link

Choose a reason for hiding this comment

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

White space change, please remove.

*
* @author Y.Voutsinas, CERN
* @date March, 2017
* @version $Id: $
Copy link

Choose a reason for hiding this comment

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

$Is is a SVN tag

@gaede
Copy link
Contributor

gaede commented Mar 24, 2017

Could we not simply have:

      /// enum for encoding the sensor type in typeFlags
      enum LayoutType{
	BarrelLayout=0,
	EndcapLayout,
        ConicalLayout
      };

      /// type of layout: BarrelLayout, EndcapLayout or ConicalLayout
      LayoutType layoutType ;

      /** extent of the calorimeter in the r-z-plane [ rmin, rmax, zmin, zmax, rmin2, rmax2 ] in mm.
         * where rmin2, rmax2 are the radii at zmax for the ConicalLayout and not used else.
         */
      double extent[6] ;

@petricm
Copy link

petricm commented Mar 24, 2017

I agree, and then you can use ConicalLayout in the overloading of <<

@Voutsi
Copy link
Contributor Author

Voutsi commented Mar 24, 2017

Thanks for your suggestions, I modified accordingly.

@gaede gaede merged commit 8b5c50b into AIDASoft:master Mar 24, 2017
@petricm
Copy link

petricm commented Mar 25, 2017

@Voutsi please modify(edit comment) in the initial comment in the release notes tag to reflect the actual change.

@Voutsi
Copy link
Contributor Author

Voutsi commented Mar 26, 2017

release notes updated

@Voutsi Voutsi deleted the FCCee branch August 8, 2017 16:08
@Voutsi Voutsi restored the FCCee branch August 8, 2017 16:19
@Voutsi Voutsi deleted the FCCee branch August 11, 2017 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants