Fix bugs in v10 upgrade. #501
Conversation
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Looks good to me.
Left comments anyway because it's still me.
| fFSPPitch0.push_back(sbn::GetPitch(wireReadout, sce, start_loc, start_dir, wireReadout.Plane(geo::PlaneID(firstTPCID, 0)).View(), geo::PlaneID(firstTPCID, 0), true, true)); | ||
| fFSPPitch1.push_back(sbn::GetPitch(wireReadout, sce, start_loc, start_dir, wireReadout.Plane(geo::PlaneID(firstTPCID, 1)).View(), geo::PlaneID(firstTPCID, 1), true, true)); | ||
| fFSPPitch2.push_back(sbn::GetPitch(wireReadout, sce, start_loc, start_dir, wireReadout.Plane(geo::PlaneID(firstTPCID, 2)).View(), geo::PlaneID(firstTPCID, 2), true, true)); |
There was a problem hiding this comment.
Was this an existing bug, or a new one?
There was a problem hiding this comment.
I needed to change the GetPitch interface in order to fix introduced by the v10 transition.
| double angleToVert = wireReadout.WireAngleToVertical(view, tpc.ID()) - 0.5*::util::pi<>(); | ||
| const geo::GeometryCore *geo = lar::providerFrom<geo::Geometry>(); | ||
|
|
||
| double angleToVert = wireReadout.WireAngleToVertical(view, plane_id) - 0.5*::util::pi<>(); |
There was a problem hiding this comment.
By the way, this is the direction from geo->Plane(plane_id).GetIncreasingWireDirection(), which would make cosgamma a simple std::abs(geo::vect::Dot(dirToVert, dir_w)).
There was a problem hiding this comment.
That sounds right. I prefer this method, because other pieces of code (like the Calorimetry) also use it to compute the pitch. So more people would notice if something breaks.
|
|
||
| // Assume extremum for wire number depending on z coordinate | ||
| if (position[2] < m_geometry->TPC().HalfLength()) wire = 0; | ||
| if (position[2] < m_geometry->TPC().ActiveLength()*0.5) wire = 0; |
There was a problem hiding this comment.
| if (position[2] < m_geometry->TPC().ActiveLength()*0.5) wire = 0; | |
| if (position[2] < m_geometry->TPC().ActiveHalfLength()) wire = 0; |
BTW, this assumes the center of the TPC is at 0. That is not guaranteed — and I can just hope that's the case in SBND.
There was a problem hiding this comment.
This is a good point and actually might be an issue for people using SPINE in SBND. Maybe @bear-is-asleep could comment?
There was a problem hiding this comment.
The center wrt the drift coordinate is at x = 0.
|
trigger build |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
❌ CI build for ICARUS Failed at phase build ICARUS on slf7 for e26:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build ICARUS phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests SBND phase logs parent CI build details are available through the CI dashboard |
No description provided.