Feature #1485

Last SciFi cross link in the new API

Added by Dobbs, Adam about 8 years ago. Updated about 8 years ago.

Target version:
Start date:
13 June 2014
Due date:
% Done:


Estimated time:
New Issue


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.


maus_output.json (1.28 MB) maus_output.json Dobbs, Adam, 16 June 2014 15:28
maus_output.root (283 KB) maus_output.root Dobbs, Adam, 16 June 2014 15:28

Updated by Dobbs, Adam about 8 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 about 8 years ago

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
./ --configuration_file datacard_mc_helical

Updated by Rogers, Chris about 8 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/ --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/", 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 about 8 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 about 8 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:

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 about 8 years ago

Have not been able to reproduce Chris' error, Durga do you see it?


Updated by Rogers, Chris about 8 years ago

I note that the relevant branch is commented in the latest trunk:

    RegisterValueBranch("covariance", &_matrix_proc,
                        &SciFiTrackPoint::set_covariance, false);
//     RegisterTRefArray("clusters", &_cluster_tref_proc,      <<<<<<<<<<<< COMMENTED HERE
//                       &SciFiTrackPoint::get_clusters, &SciFiTrackPoint::set_clusters, true);
} // ~namespace MAUS

Updated by Dobbs, Adam about 8 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 about 8 years ago

Tested passed, pushed to merge branch. All should now work, except for the broken cross link.


Updated by Rogers, Chris about 8 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 about 8 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/ [0x7f65fc9cb49a]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcbc682c]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcdaf6c2]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcdb196f]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd7f56a]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcdac0a3]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd6d603]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd7a743]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd6b9c3]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd0b3fa]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fcd04003]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fccfed13]
/home/cr67/MAUS/maus_merge/build/ [0x7f65fce0c939]


Updated by Rogers, Chris about 8 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 about 8 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 about 8 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

Pushed. Closing the issue.

Also available in: Atom PDF