Feature #1485
Last SciFi cross link in the new API
100%
Description
I have now finished porting the scifi code to the new API, with just one exception. We currently have a scifi event, which contains digits, clusters, spacepoints, straight and helical pat rec tracks, and kalman tracks. The pat rec tracks contain cross links to spacepoints, spacepoints contain cross links to clusters, and clusters contain cross links to digits. All this seems to work happily with the new API, which I have tested by looking at cout's and with unit tests. The ROOT output also seems good.
The kalman tracks also contain trackpoints (it owns them, like the scifi event owns the objects within it). Each trackpoint in turn contains one cross link to a single cluster. This cross link I cannot get to work in the ROOT output, it keeps turning up as a zero size TRefArray (the working cross links appear as populated TRefArrays). I have checked the output at the end of the tracker recon mapper, and all the trackpoint - cluster cross links seem fine there, and also written a unit test which seems to agree. Further, when I output to JSON, that seems to contain the correct cross links too, leaving only the ROOT output as a problem.
My current efforts are in the merge branch, so anyone can look (the loss of the cross link only affects analysis not recon). One possible issue is that I have had to implement this single cross link as a TRefArray, rather than a TRef, as TRef does not have a JsonCppProcessor written. But I don't think that should matter...
Have spent a lot of time wracking my brain over this, so would appreciate some help.
Files
Updated by Dobbs, Adam almost 9 years ago
Extra info. Just checked the trackpoint - cluster cross link in a reducer immediately after the tracker recon mapper, and the cross link is broken there too.
Updated by Dobbs, Adam almost 9 years ago
- File maus_output.json maus_output.json added
- File maus_output.root maus_output.root added
Here is some output to illustrate, a json file where the cross link are in place, and a root file generate directly from the json, with the cross link broken. The command to reproduce the json or root file from simulation is
cd bin/user/scifi ./simulate_scifi.py --configuration_file datacard_mc_helical
Updated by Rogers, Chris almost 9 years ago
Is there an easy way to change my datastructure so I can reproduce the problem? I tried to run from trunk
~/MAUS/maus_merge/bin/utilities/json_to_root.py --input_json_file_name maus_output.json --output_root_file_name maus_test.root
... snip ...
Traceback (most recent call last):
File "/home/cr67/MAUS/maus_merge/src/common_py/ErrorHandler.py", line 159, in HandleCppException
raise CppError(error_message)
ErrorHandler.CppError: In branch recon_events
In branch sci_fi_event
In branch tracks
In branch trackpoints
Failed to recognise all json properties clusters at ObjectProcessor<ObjectType>::JsonToCpp
oops, but obviously trackpoint/clusters are disabled so this makes sense...
Updated by Rajaram, Durga almost 9 years ago
Adam, I took lp:~phuccj/maus/merge_candidate -- is that the right one?
Do you have a macro handy for poking into the SciFi structure in the ROOT file?
Updated by Dobbs, Adam almost 9 years ago
Hmmmm, I'm a bit confused, all the necessary changes should be in the trunk itself, with no modification to the data structure required. In fact I would be happy for it to go in to a release, the last broken cross link not withstanding. If this were broken, I would expect my scifi integration test to be failing and breaking Jenkins (this exactly one of the scenarios it should protect against).
Durga, yes, if you go to:
bin/user/scifi/cpp make all ./scifi_analysis ../maus_output.root
That should provide an example analysis of scifi data illustrating how to track through the data structure.
Updated by Dobbs, Adam almost 9 years ago
Have not been able to reproduce Chris' error, Durga do you see it?
Updated by Rogers, Chris almost 9 years ago
I note that the relevant branch is commented in the latest trunk:
RegisterValueBranch("covariance", &_matrix_proc,
&SciFiTrackPoint::covariance,
&SciFiTrackPoint::set_covariance, false);
// RegisterTRefArray("clusters", &_cluster_tref_proc, <<<<<<<<<<<< COMMENTED HERE
// &SciFiTrackPoint::get_clusters, &SciFiTrackPoint::set_clusters, true);
}
} // ~namespace MAUS
Updated by Dobbs, Adam almost 9 years ago
Confirmed, thanks Chris. Running tests on the fix, will push when they finish in a hour or so and update you.
Updated by Dobbs, Adam almost 9 years ago
Tested passed, pushed to merge branch. All should now work, except for the broken cross link.
Updated by Rogers, Chris almost 9 years ago
So the call history goes like this, where pointers are all for the TRefArray
SciFiTrackPoint::SciFiTrackPoint() 0x18606c0 SciFiTrackPoint::set_clusters(TRefArray*) 0x1860a30 SciFiTrackPoint::SciFiTrackPoint(const SciFiTrackPoint &) 0x1861ce0 TRefArrayResolver::ResolveReferences #recon_events/0/sci_fi_event/clusters/0 0x1860a30 0
We initialise the SciFiTrackPoint; JsonCppProcessor calls set_clusters; then there is a copy constructor call; then TRefArrayResolver attempts to resolve the reference. Only TRefArrayResolver doesn't know about the copy constructor call, so it attempts to ResolveReferences on the wrong TRefArray*. So the answer is to figure out who is calling a copy constructor before reference resolution phase...
Updated by Rogers, Chris almost 9 years ago
And the winner is ... SciFiTrack::set_scifitrackpoints. Normally these Set functions should take ownership of memory pointed to, rather than making a deep copy. So SciFiTrack::set_scifitrackpoints(std::vector<SciFiTrackPoints>*) should become the owner of this memory. It was probably a memory leak as well.
This is a limitation of doing a two-phase reference resolution (the alternative I guess is the ROOT way where we require special data structs for cross links).
Stack trace (from SciFiTrackPoint copy ctor):
/home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS15SciFiTrackPointC1ERKS0_+0x3ba) [0x7f65fc9cb49a] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS10SciFiTrack20set_scifitrackpointsESt6vectorIPNS_15SciFiTrackPointESaIS3_EE+0xac) [0x7f65fcbc682c] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS17ObjectProcessorNS9ValueItemINS_10SciFiTrackESt6vectorIPNS_15SciFiTrackPointESaIS5_EEE12_SetCppChildERKN4Json5ValueERS2_+0x162) [0x7f65fcdaf6c2] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS15ObjectProcessorINS_10SciFiTrackEE9JsonToCppERKN4Json5ValueE+0x6f) [0x7f65fcdb196f] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS21PointerArrayProcessorINS_10SciFiTrackEE9JsonToCppERKN4Json5ValueE+0x17a) [0x7f65fcd7f56a] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS17ObjectProcessorNS9ValueItemINS_10SciFiEventESt6vectorIPNS_10SciFiTrackESaIS5_EEE12_SetCppChildERKN4Json5ValueERS2_+0xa3) [0x7f65fcdac0a3] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS15ObjectProcessorINS_10SciFiEventEE9JsonToCppERKN4Json5ValueE+0x63) [0x7f65fcd6d603] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS17ObjectProcessorNS11PointerItemINS_10ReconEventENS_10SciFiEventEE12_SetCppChildERKN4Json5ValueERS2_+0x163) [0x7f65fcd7a743] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS15ObjectProcessorINS_10ReconEventEE9JsonToCppERKN4Json5ValueE+0x63) [0x7f65fcd6b9c3] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS21PointerArrayProcessorINS_10ReconEventEE9JsonToCppERKN4Json5ValueE+0x17a) [0x7f65fcd0b3fa] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS17ObjectProcessorNS11PointerItemINS_5SpillESt6vectorIPNS_10ReconEventESaIS5_EEE12_SetCppChildERKN4Json5ValueERS2_+0x163) [0x7f65fcd04003] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZN4MAUS15ObjectProcessorINS_5SpillEE9JsonToCppERKN4Json5ValueE+0x63) [0x7f65fccfed13] /home/cr67/MAUS/maus_merge/build/libMausCpp.so(_ZNK4MAUS21JsonCppSpillConverter8_convertEPKN4Json5ValueE+0x79) [0x7f65fce0c939]
Updated by Rogers, Chris almost 9 years ago
- Assignee changed from Rogers, Chris to Dobbs, Adam
Spoke to Adam on the phone - he said that he wanted to fix it...
Updated by Dobbs, Adam almost 9 years ago
That did it, the last cross link now works. Thanks Chris. The scifitrack.set_scifitrackpoints now only does a shallow copy, and have updated the SciFiEvent copy constructor so that it performs the deep copy of the scifitrack trackpoints needed there.
Additionally I have switched the cross link container for the trackpoint cluster from being a TRefArray* to a TRef*. I had to write special getters and setters which return and take TObject pointers to get the JsonCppProcessor to work, but now everything seems happy.
Just running tests, then will push to the merge branch...
Updated by Dobbs, Adam almost 9 years ago
- Status changed from Open to Closed
- % Done changed from 0 to 100
Pushed. Closing the issue.