search

JeffersonLab / hpstr

Heavy Photon Search Toolkit for Reconstruction
2 stars 19 forks source link

Upgrade to c17 standard in preparation for upgrades to the processing module #93

Closed omar-moreno closed 4 years ago

omar-moreno commented 4 years ago

At the moment, there are issues with being able to read both an LCIO file and a newly created collection. The reason for this is because each processor is currently accessing the input/output tree directly instead of going through the EventFile which took care of buffering all collections.

I came across this issue when trying to do the following:

Run two processors at a time configured to run in LCIO mode. The first processor converts LCIO mc particles hits to MCParticles while the second processor converts LCIO tracker hits to MCTrackerHits. However, the second processor also wants to add a reference to the MCParticle which requires access to the newly created MCParticle collection. Currently, you can't easily do this in a clean way.

Handling everything through an event bus will fix this and being able to use std::variant would make it easy to implement.

mholtrop commented 4 years ago

Omar, can this be discussed in the software meeting? I understand that you like C++17, but I am quite concerned that pushing to C++17 will make hpstr even less accessible to new people. I am not convinced that std::variant is really a needed feature to achieve the enhancement you are looking for.

omar-moreno commented 4 years ago

Sure.

On Mon, May 4, 2020, 6:37 AM Maurik Holtrop notifications@github.com wrote:

Omar, can this be discussed in the software meeting? I understand that you like C++17, but I am quite concerned that pushing to C++17 will make hpstr even less accessible to new people. I am not convinced that std::variant is really a needed feature to achieve the enhancement you are looking for.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/JeffersonLab/hpstr/issues/93#issuecomment-623468993, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4JMXCQNCSN5PI6VAEBKWTRP3ALJANCNFSM4MYOFY7A .

mholtrop commented 4 years ago

Looks like this can be closed. I am currently compiling and running hpstr with -std=c++17 on MacOS without issue. On Ubuntu 20.04 and Centos 7 with gcc 9.3 the LCIO version needs to be "master" or the ldmx-sw fork should be used. No modifications to hpstr are needed though. Can you close this issue, since there is a separate one for the event bus?