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

Root interface changes #428

Merged
merged 2 commits into from
Aug 7, 2018
Merged

Conversation

andresailer
Copy link
Member

@andresailer andresailer commented Aug 6, 2018

BEGINRELEASENOTES

ENDRELEASENOTES

@ianna
Copy link
Contributor

ianna commented Aug 6, 2018

@mrodozov - FYI

const Double_t* t = matrix.GetTranslation();
if ( matrix.IsRotation() ) {
const Double_t* r = matrix.GetRotationMatrix();
return Transform3D(r[0],r[1],r[2],t[0]*MM_2_CM,
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a lot of conversions here. It looks like the MM_2_CM and RAD_2_DEGREE assume that the input units are mm and rad. Should it be a user who defines which units to use and if they are not the DD4hep default, convert them?

Copy link

Choose a reason for hiding this comment

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

The user should always use DD4hep units, when addressing/talking to DD4hep. There some exceptions to this rule, e.g. when interfaces of underlying classes are exposed, there you should use or expect ROOT or Geant4 conventions.

@MarkusFrankATcernch
Copy link
Contributor

Updates are fine. We requested them in ROOT,
so we should be prepared to update our code accordingly.

@petricm petricm merged commit b0ca8dc into AIDASoft:master Aug 7, 2018
@andresailer andresailer deleted the rootInterfaceChanges branch August 7, 2018 11:01
Copy link
Contributor

@MarkusFrankATcernch MarkusFrankATcernch left a comment

Choose a reason for hiding this comment

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

in DDCore/src/plugins/LCDDConverter.cpp
I would simply put:
TGeoMatrix linv = lm->Inverse();
and
TGeoMatrix rinv = rm->Inverse();

Modern compilers are efficient on these statements. There should be no need to use a reference.

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.

ROOT TGeoMatrix interface change
5 participants