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

Support arbitrary readout orientation #85

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

lobis
Copy link
Member

@lobis lobis commented May 31, 2023

lobis Medium: 137

Currently the readout plane only works correctly if the orientation is in the z direction.

TVector2 rot = coords.Rotate(-fModuleRotation * TMath::Pi() / 180.);

return rot;
return p - TVector2(fModuleOriginX, fModuleOriginY);
Copy link
Member

Choose a reason for hiding this comment

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

why rotation has disappeared?


return coords;
inline TVector2 TransformToReadoutCoordinates(const TVector2& p) {
return p + TVector2(fModuleOriginX, fModuleOriginY);
Copy link
Member

Choose a reason for hiding this comment

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

rotation?

Copy link
Member

Choose a reason for hiding this comment

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

It should be TransformToModuleCoordinates and TransformToPlaneCoordinates. The module coordinates define the (0,0) at the left-bottom corner.

TVector2 GetPhysicalCoordinates(const TVector2& p) {
return TransformToPhysicalCoordinates(p.X(), p.Y());
}
TVector2 GetPhysicalCoordinates(const TVector2& p) { return TransformToReadoutCoordinates(p); }
Copy link
Member

Choose a reason for hiding this comment

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

Should be GetPlaneCoordinates now.

TVector3 fPlaneAxisX; ///< The first coordinate vector of the plane in absolute coordinates.
/// < This vector is contained in the plane and corresponds to the first local
/// coordinate (1,0)
TVector3 fPlaneAxisY; ///< The first coordinate vector of the plane in absolute coordinates.
Copy link
Member

Choose a reason for hiding this comment

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

There is no need for fPlaneAxisY, this vector is the result of fPlaneAxisX times fPlaneVector.

Also we said to rename them to fAxisY fNormal

Copy link
Member Author

Choose a reason for hiding this comment

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

the Y vector is the second coordinate vector in the plane, it's true that it is redundant as it can be obtained via the cross product (but it's different from the normal).

return GetModuleByID(mod)->GetDistanceToModule(pos);
}

TVector2 GetPositionInReadoutPlane(const TVector3& position) const;
Copy link
Member

Choose a reason for hiding this comment

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

Must be renamed to GetPosition

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see now it is not the position, but the projection. So the method could be named GetProjectedCoordinates.

@@ -143,13 +158,11 @@ class TRestDetectorReadoutPlane : public TObject {

Int_t isZInsideDriftVolume(Double_t z);

Int_t isZInsideDriftVolume(TVector3 pos);
Int_t isZInsideDriftVolume(const TVector3& pos);
Copy link
Member

Choose a reason for hiding this comment

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

This method must be updated, perhaps now the method should be named IsBetweenCathodeAndReadout

@lobis
Copy link
Member Author

lobis commented Jun 7, 2023

I will update everything later today and fix the conflicts

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.

2 participants