Home » PANDA » PandaRoot » Tracking » WirepointHitPolicy detplane implementation
WirepointHitPolicy detplane implementation [message #8080] |
Mon, 16 March 2009 13:55 |
Anonymous Poster
|
|
From: 141.39.167*
|
|
Hi,
when I replied to Ola's thread, I stumbled over something... In the wirepoint policy, the detPlane method is not implemented. But if you inherit from RecoHitIfc<WirePointHitPolicy> you should implement, hot to get the detPlane in this place. I know it is again virtual in RecoHitIfc, and indeed you implement this feature in PndSttRecohit. But this is not the idea of genfit. If someone else wants to use you policy, he has to reimplement this code, which is of course the only complicated code for a wire hit.
If you put the code to WirepointHitPolicy::detPlane(), and dont further overwrite it in PndSttRecoHit, it will be called through RecoHitIfc (please have a look at the tiny file genfit/RecoHitIfc.h).
So, could you please change that? Or is there a problem here I overlooked. If it is changed in all file, I would like to make this method in RecoHitIfc non-virtual, to avoid these situations in the future.
Cheers, Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8082 is a reply to message #8080] |
Mon, 16 March 2009 14:16 |
Anonymous Poster
|
|
From: 141.39.167*
|
|
Hi again,
I forgot to mention that the other place where this appears is in PndDchRecoHit2. It looks like the same code was used in both cases.
If you implement this code in WirepointHitPolicy, then also you you dont need to reimplement getDetPlane at all. The detPlane method of the policy is called in RecoHitIfc.
Again, I want to beg your pardon that this i so complex and you dont have any docu. I am working on this! Please help me to get some consistency in the package, by moving this implementation.
If you have any more questions, please do not hesitate to bug me all you want on this!
Christian
|
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8087 is a reply to message #8086] |
Mon, 16 March 2009 15:54 |
Anonymous Poster
|
|
From: 141.39.167*
|
|
Hi Lia and Ola,
thanks for pointing out the problems with my idea. I have to think about whether it would be wise to have such a function like the AbsTrackRep::extrapolateToPoca (which is called by virtualDetectorPlane)...
Actually it might well be that I remove getVirtualDetectorPlane all together, because it just calls AbsTrackRep::extrapolateToPoca....
Probably the best way to solve this would be to add a method to AbsTrackRep which is extrapolateToLine (which is clear what it does) and then call this from WirepointHitPolicy::detplane(). Your Poca code would then be implemented in GeaneTrackRep.
What do think about this solution?
If there is still slight modifiactions to the standard behavior necessary in DchHit, one could override detPlane from the policy, call the super-class methid, and then modify the result before returning the result.
Please let me know what you think!
Christian
|
|
|
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8092 is a reply to message #8091] |
Mon, 16 March 2009 18:46 |
Anonymous Poster
|
|
From: *pool.einsundeins.de
|
|
Hi,
thanks for the info. I Have to think about that distance problem. But as for the choice of the origin for the plain, it is completely arbitrary, isnt it? That would mean that one could just pick one of the two choices for both detectors.
Cheers, Christian
|
|
|
|
new in AbsTrackRep: extrapolateToLine and consequences on WirepointHitPolicy [message #8095 is a reply to message #8093] |
Mon, 16 March 2009 23:10 |
Anonymous Poster
|
|
From: *pool.einsundeins.de
|
|
Hi,
I introduced a new virtual method in AbsTrackRep, which is
virtual TVector3 extrapolateToLine(const TVector3& point1,
const TVector3& point2,
TMatrixT<double>& statePred,
TMatrixT<double>& covPred,
DetPlane& planePred);
It has a default implementation in AbsTrackRep.cxx, so all TrackReps which dont have this feature are still fine.
Could you please put your extrapolate to line code in GeaneTrackRep::extrapolateToLine, which you would have to put in GenaeTrackRep.h and .cxx? The return value TVector3 would be the point (of closest approach).
I still have to think about your distance problem, but I am sure we solve that special case for the straws to still share the policy::detPlane with Dch.
Putting this extrapolateToLine to GeaneTrackRep is just the first step.
Why I am doing all this: To combine code which does the same all in one place, and make the structure cleaner and more understandable. I am sorry for all the work I cause you.
Thank you, Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8096 is a reply to message #8080] |
Mon, 16 March 2009 23:25 |
Anonymous Poster
|
|
From: *pool.einsundeins.de
|
|
Hi again,
I have a very simple solution for your distance cut: Put a
protected:
double _maxDistance
in WirepointHitPolicy. Then make a WirepointHitPlicy which loks something like this
const DetPlane&
SpacepointHitPolicy::detPlane(AbsRecoHit* hit, AbsTrackRep* rep)
{
TMatrixT<double> rawcoord = hit->getRawHitCoord();
//I dont know which of the 8 params is for the wire points
TVector3 point1(rawcoord[0][0],rawcoord[1][0],rawcoord[2][0]);
TVector3 point2(rawcoord[3][0],rawcoord[4][0],rawcoord[5][0]);
int dimension = rep->getDim();
TMatrixT<double> statePred(dimension,1);
TMatrixT<double> covPred(dimension,dimension);
//note that _plane is defined in AbsTrackRep
TVector3 poca=rep->extrapolateToLine(point1,point2,statePred,covPred,_plane);
/*C. Hoeppner, March 09: I am not entirely sure that these two calls
are needed, but they dont hurt for sure. Something happens here
to the orientation of u and v, so keep it.*/
TVector3 m=_plane.getNormal();
_plane.setNormal(m);
//now calculate distance of poca to line between point1 and point2
double distance;
if(distance>_maxDistance) {
cout << "vpf greater than maxValue" << endl;
FitterException exc("distance vpf-wire larger than maxValue", __LINE__,__FILE__);
throw exc;
}
return _plane;
}
For the DCH where you dont want this distance cut, just set the _maxDistance (do this in the contructors of the recoHits) to a very large value. Or you can use a bool flag to deactivate this feature. This you would also be set in the ctors.
About the origin of the plane: can you come to a common solution? Is Lia's solution OK for you Ola?
Thanks for your support!
Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8097 is a reply to message #8093] |
Tue, 17 March 2009 10:21 |
Aleksandra Wronska
Messages: 38 Registered: May 2006 Location: Cracow
|
continuous participant |
From: *if.uj.edu.pl
|
|
Dear Lia and Christian,
I have no objections about introducing _maxDistance in WirepointHitPolicy. It's the easiest solution, isn't it?
Concerning the origin of detPlane, the answer for now is "I do not know", I am afraid. I need a few hours to look for a reasonable solution for dch to use with a fixed plane origin. Any suggestions from Lia (as the original author of the code) are welcome
What I have in mind (without a clear idea of implemenation yet) is that I could set the Z coordinate of the DchRecoHit2 (checking previously that it was not set before, such that the same code works for you, too) to the Z of poca on the wire, calculated with respect to the wire centre. Does it have any chance to work, Lia?
This would require an extra function for setting Z of a RecoHit, but that's a minor thing, isn't it?
cheers,
ola
|
|
|
|
|
Re: new in AbsTrackRep: extrapolateToLine and consequences on WirepointHitPolicy [message #8103 is a reply to message #8095] |
Tue, 17 March 2009 18:13 |
Lia Lavezzi
Messages: 291 Registered: May 2007 Location: Torino
|
first-grade participant |
From: *pv.infn.it
|
|
Hi,
I am trying to "move" the code from the stt reco hit to the extrapolateToLine, but there are some problems:
1) in the stt reco code we only find the point of closest approach to the wire and from this we build the detector plane, but looking at the extrapolateToLine function I see that we want a complete extrapolation, with the state/cov/detPlane predictions... and this brings me to the second problem...
2) the easiest way to do this is to use the geane PropagateToVirtualPlaneAtPCA: this function calculates the point of closest approach to the wire, builds the plane (in the STT usual way) and performs the propagation to this plane. This would give the right result, but doing things this way we would have the propagation from the starting point to the detPlane twice for each kalman step (one here when the plane is built, the other when the Kalman extrapolation is performed). I had also a quick look into the extrapolateToPoca and I see the same problem there...
Given these two considerations, wouldn' t it be better to have here (to be used in the detector plane determination) two findPocaToPoint/ToLine functions, which only find the poca, in addition to (or in substitution of) the two extrapolateToLine/ToPoca functions ? They could also give the detector plane or this could be left in the detPlane(...) functions.
Another solution could be to leave the extrapolateToPoca/ToLine, but to avoid to repeat it with the usual extrapolate call into Kalman::processHit.
What do you think?
I could also write the function with the PropagateToVirtualPlaneAtPCA solution for now and leave the decision to a later stage...
Ciao,
Lia.
|
|
|
|
Re: new in AbsTrackRep: extrapolateToLine and consequences on WirepointHitPolicy [message #8105 is a reply to message #8104] |
Wed, 18 March 2009 11:51 |
Anonymous Poster
|
|
From: *e18.physik.tu-muenchen.de
|
|
Hi Lia,
you are absolutely right that we do some extrapolations twice. I thought a lot about this problem, and I can not come up with a general solution to it. Here is the biggest problem with it:
If you would only want to do it once, you would have to return the statePred and CovPred together with the DetPlane in AbsRecoHit::getDetPlane(). This is not yet a problem. But, in your hit policy::detPlane, where you implement this, you have to make some call to an extrapolateToLine or so of AbsTrackRep. Now if this is supposed to give you also the state and Cov, it needs to fix already the DetPlane, because state and cov can only be defined in a plane.
This is in general impossible, since it would mean that everytime you want to change your definition of the DetPlane for a wire hit or so, you would have to change track reps, and then different detectors would need extrapolate1,2,3,... functions. In short it would be a mess!
My opinion is this: We make a an extrapolateToLine and also to Poca, which does not give the state,cov and plane results and just returns the POCA. We live with the fact, that we do some extrapolations twice. But keep in mind that they are not really the same, in the final extrapolation you fixed your plane geometry, and it is clear that then you have to do another extrapolation.
Please remember, that we are paying this minor price for a great deal of flexibility which we gain! And I think that is exactly what we need at this early stage of our experiment. We can still easily test and interchange different track models and hit geometries without any changes to the core code of the tracking! To my knowledge (and I did quite a lot of research on finding something) there is no other tracking system which could even handle the STT and TPC together with everything else in geometry we have it.
Lia, what do you think of my proposal for changing what the extrapolateToLine should do?
Cheers, Christian
|
|
|
Re: new in AbsTrackRep: extrapolateToLine and consequences on WirepointHitPolicy [message #8107 is a reply to message #8105] |
Wed, 18 March 2009 13:23 |
Lia Lavezzi
Messages: 291 Registered: May 2007 Location: Torino
|
first-grade participant |
From: *pv.infn.it
|
|
Hi,
ok, please, let me shortly summarize things, because I' m getting lost in all this discussion and I want to understand things well
Let' s consider for now the extrapolateToPoca (just because it is already implemented, and the extrapolateToLine will be almost the same):
- NOW it does the following (by calling the PropagateToVirtualPlaneAtPCA): it extrapolates to a very large track length, finds the point of closest approach, builds here its own detector plane, finally re-extrapolates to this detector plane and gives the results, which are the state/cov/detPlane/poca.
- Your idea is to rewrite the extrapolateToPoca in order not to call the PropagateToVirtualPlaneAtPCA and to just have one extrapolation (the one to a very large track length), which only finds the point of closest approach, without filling any PREDICTED state/cov/detPlane.
If so, this is the same idea I have, so I agree
In this case we will have (that' s true!) an extra extrapolation, but is really necessary in order to find the poca, so we just have to keep it.
Please correct me if I misunderstood something...
Ciao,
Lia.
|
|
|
Re: new in AbsTrackRep: extrapolateToLine and consequences on WirepointHitPolicy [message #8109 is a reply to message #8107] |
Wed, 18 March 2009 13:37 |
Anonymous Poster
|
|
From: *natpool.mwn.de
|
|
Hi Lia,
yes this exactly what I meant! Just one more question before I make the changes:
When I change the argument list of extrapolateToPoca and ToLine, could you please make the proper modifications to GeaneTrackRep::extrapolateToPoca to remove the not anymore needed functionality. You are much more of an expert than me in Genae and dont think it will be a lot of work for you, will it?
So, I will change the argument list now. After you had a chance to remove the functionality from extrToPoca I will test immediately if everything still works.
Sounds like a plan?
Thanks for the discussion, Christian
|
|
|
|
|
changes in PndDchKalmanQA [message #8112 is a reply to message #8080] |
Wed, 18 March 2009 15:32 |
Anonymous Poster
|
|
From: *natpool.mwn.de
|
|
Hi Ola,
I am changing the argument list for AbsTrackRep::extrapolateToPoca. One of your files, PndDchKalmanQATask.cxx will require a change. I think I can not submit it, but if I submit the rest of the changes, the trunk wont compile anymore. Are you there to check in a diff I can send you (shortly) after I check in my code?
Cheers, Christian
|
|
|
Re: changes in PndDchKalmanQA [message #8113 is a reply to message #8112] |
Wed, 18 March 2009 16:13 |
Anonymous Poster
|
|
From: *natpool.mwn.de
|
|
Hi Ola,
Mohammad was so kind to give me write access to dch/DchFitting. I promise I will give these rights up as soon as this restructuring is done. I wont change the functionality of your code, just the formalities.
Lia, I also have write access temporarily to stt/sttreco for the same reason.
Cheers, Christian
|
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8115 is a reply to message #8080] |
Wed, 18 March 2009 17:21 |
Anonymous Poster
|
|
From: *e18.physik.tu-muenchen.de
|
|
Hi Lia,
the changes are now commited. You can overwrite extrapolateToLine in GenaeTrackRep.
You have to return two values by reference: the poca and the direction of the track in the poca. This is needed I needed to calculate the detPlane, at least in my case. If you dont need it for the STT, please implement it in the trackRep anyways, for future use, afterall it should be easy.
Could you also have a look in extrapolateToPoca? Maybe it can be cleaned up a little bit. I already changed it to do what I need, and I tested that it is still doing fine.
I am out for today. Thanks for all your help!
Cheers, Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8118 is a reply to message #8115] |
Thu, 19 March 2009 13:35 |
Lia Lavezzi
Messages: 291 Registered: May 2007 Location: Torino
|
first-grade participant |
From: *pv.infn.it
|
|
Hi Christian,
I implemented the GeaneTrackRep::extrapolateToLine but before putting it on the svn I have one question: in STT case we need to know the point of closest approach on wire in addition to the point of closest approach on the track (the usual poca) to build the plane, while we don' t care about the direction of the track in poca, can we change the TVector3 from dirInPoca to poca_onwire (or something like that) ?
Concerning the extrapolateToPoca I have to look at it more deeply, but since you don' t need the state/cov at the poca something like the extrapolateToLine (but with pca = 1) should work (to only find the poca, without the need to call the "Propagate" function which performs the actual propagation)... I only see a problem in finding the direction of the track at the point of closest approach... I need to think about this a little, to see if it is possible to get it without any extra extrapolation (a part the one to find the poca). I will let you know.
For the extrapolateToLine, my implementation is ready, with the change dirInPoca --> poca_onwire: I tested it with my stt reco hit and it works fine, so if you give me the "ok" I can put it on the svn.
Ciao,
Lia.
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8120 is a reply to message #8118] |
Thu, 19 March 2009 14:11 |
Anonymous Poster
|
|
From: *natpool.mwn.de
|
|
Hi Lia,
well the direction in the POCA, I think I already have in now in extrapolateToPoca. Maybe you can take a look, but since all my fist work, I pretty satisfied with the situation.
I see you point on needing poca_onwire, and I want to add it. But one question: Do you have the info about the direction of the track in the POCA (maybe you want to look in extrapolateToPoca)? If so, I would like to add your parameter as a third return value and not replace it (so the argument list would stay with an additional TVector3& poca_onwire).
Chhers, Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8121 is a reply to message #8120] |
Thu, 19 March 2009 16:42 |
Lia Lavezzi
Messages: 291 Registered: May 2007 Location: Torino
|
first-grade participant |
From: *47-151.net24.it
|
|
As the code is written now, I don' t have the info on the track direction in poca, but I think I could get it by writing a code more similar to the extrapolateToPoca (i.e. by adding the additional propagation): do we really need this info?
That' s true, in the extrapolateToPoca you have the track direction because you extrapolate the track to the poca, but I thought we wanted to get rid of the statePred (and covPred) in the extrapolateToPoca/ToLine in order to avoid to call the Propagate function there, did we? ...I' m a little confused...
I attach to this message the GeaneTrackRep.cxx with my implementation of the extrapolateToLine, just to show you how I would do things (at least for the extrapolateToLine)... but anyway it would not be so difficult for me to change the code and make it similar to the extrapolateToPoca.
I' d like to find the track direction without the need of performing the propagation, but I admit the most simple way is (as you did) to perform the additional propagation after having found the poca; in this case, however, we could do things in a slighlty different way: now we use the PropagateToVirtualPlaneAtPCA and then a virtual detector plane is built internally (in Propagate): this would be ok if we wanted to consider this plane as detPlane, but we don' t want to do this! We want to build our own detPlane (in the appropriate function). So actually, to find the direction, it would be enough to perform a propagation to the track length (and not to the plane) which corresponds to the found poca (without the need to build the plane)... I could look in FairGeanePro, if this is already feasible.
So if we want the direction also for the extrapolateToLine, I could provide a function similar to the extrapolateToPoca in order to have a preliminary implementation and then think about some modifications to make it faster... what do you think?
Lia.
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8122 is a reply to message #8121] |
Thu, 19 March 2009 16:54 |
Anonymous Poster
|
|
From: *natpool.mwn.de
|
|
Hi Lia,
yes I think that we need this info. At least I know we do in the Poca case, because other wise I have no idea on how to orient the normal vector for the plane. I think that it is also useful to have it in the extrapolateToLine method.
I will add your third return value and commit it in a few minutes.
If you would find a way to speed things up that would be great. I think right now my top priority is still the consistent structure. But yes you are very welcome to improve things in Poca also!!
I will eave shortly.
CU tomorrow!
Christian
|
|
|
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8126 is a reply to message #8125] |
Fri, 20 March 2009 14:36 |
Anonymous Poster
|
|
From: *e18.physik.tu-muenchen.de
|
|
Hi Lia,
thanks a lot for the help in restructuring things. If you come up with something on the direction or want to discuss it, I am here....
I read again throught the argument about the difference in origin in STT and DCH. I understoof the difference now. In the STT you also have a measurement of the coordinate along the wire (by the way for curiosity: How do you plan doing that - charge sharing, timing on double sided readout?), and in the DCH case you dont have it.
But these are indeed two different things and I propose to have two policies for that.
WirepointHitPolicy for the STT
WireHitPolicy for the DCH
I guess the naming is self-explanatory. Lia, do you have write access in genfit? If so, could you implement your changed WirepoitHitPolicy and also the WireHitPolicy in which you put the code for Ola's choice of origin placement?
Then after that Ola could use WireHitPolicy and get rid of her own implementation of getDetPlane.
When we are done with this process, I will remove the virtual from getDetPlane in the RecoHitIfc to make this method non-overwriteable.
Lia, if I ask too much of you, please let me know and I will try to do some things as far as I can. Overall I think the process should be easy though.
Chers, Christian
|
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8128 is a reply to message #8127] |
Fri, 20 March 2009 18:18 |
Anonymous Poster
|
|
From: *e18.physik.tu-muenchen.de
|
|
Hi Lia,
that sounds like a good solution. Inside the WirepointHitPolicy you would save a bool value, which you fill at construction time of your recoHit, which is a switch between detectors which can and which can not measure the position along the wire. That sounds good!
You have a nice weekend, too!
Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8129 is a reply to message #8128] |
Fri, 20 March 2009 20:28 |
Anonymous Poster
|
|
From: *pool.einsundeins.de
|
|
Hi Lia,
I just checked if everything is fine now structurally and it looks great! To make it usable for the Dch, we just need the two variables maxDistance and alongWirePosFlag (or better names ) and then Ola could try out to remove the getDetPlane method from PndDhcRecoHit2 and fill the two new variables in the policy in the constructor.
I think the maxDistance should be set by default to something huge (1.E50 or whatever), so it will work by default for the DCH and needs to be set to 0.5 in the SttRecoHit ctor. For the flag for the z-pos.... I dont know what should be the default. Maybe a detector which doesnt measure it is more general. Either is fine for me though.
Bye, Christian
|
|
|
Re: WirepointHitPolicy detplane implementation [message #8132 is a reply to message #8129] |
Mon, 23 March 2009 13:32 |
Anonymous Poster
|
|
From: *e18.physik.tu-muenchen.de
|
|
Hi Ola & Lia,
on second thought, I think it is necessary to have two hit policies for detectors which can measure a position along the wire and those which can not. The reason is the dimensionality of the hit. The DCH hit is only a 1-D information. There for the correct HMatrix (OK, this has nothing to do with the policy) would be 5x1 and would be [0,0,0,1,0] assuming that the U-vector points perpendicular to the wire, like it does.
Also in the policy, where you also fix the dimensionality when you calculate the hit coordinates and covariances this should be dont correctly.
It is true that just putting the coordinate to zero with a huge errors will lead to an unbiased fit, but the number of degrees of freedom for the chi2 calculation will be wrong.
If you dont have any objections I will introduce a new policy WireHitPolicy (without the point) which will do this correctly. I would then still ask you to check the detPlane() method for your choice of detector plane before you use it.
Does that make sense? Would you have some time to test this new implementation?
Cheers, Christian
|
|
|
|
|
|
Goto Forum:
Current Time: Wed Nov 06 21:12:20 CET 2024
Total time taken to generate the page: 0.00876 seconds
|