AIDASoft / podio

PODIO
GNU General Public License v3.0
24 stars 60 forks source link

Use types of heterogeneous origins in interfaces #611

Open BrieucF opened 6 months ago

BrieucF commented 6 months ago

This:

interfaces:
  extension::TrackerHit:
    Description: "Tracker hit interface class"
    Author: "Thomas Madlener, DESY"
    Members:
      - uint64_t cellID // ID of the sensor that created this hit
      - int32_t type // type of the raw data hit
      - int32_t quality // quality bit flag of the hit
      - float time [ns] // time of the hit
      - float eDep [GeV] // energy deposited on the hit
      - float eDepError [GeV] // error measured on eDep
      - edm4hep::Vector3d position [mm] // hit position
    Types:
      - edm4hep::TrackerHitPlane
      - edm4hep::TrackerHit3D
      - extension::DriftChamberDigi

Crashes with

/afs/cern.ch/user/b/brfranco/work/public/tracketHitInterface_drift_chamber/k4RecTracker/build/DCHdigi/src/TrackCollection.cc:12:10: fatal error: extension/TrackerHit3DCollection.h: No such file or directory
   12 | #include "extension/TrackerHit3DCollection.h"

There is a workaround which is to redefine those types in the extensions and use

interfaces:
  extension::TrackerHit:
    Description: "Tracker hit interface class"
    Author: "Thomas Madlener, DESY"
    Members:
      - uint64_t cellID // ID of the sensor that created this hit
      - int32_t type // type of the raw data hit
      - int32_t quality // quality bit flag of the hit
      - float time [ns] // time of the hit
      - float eDep [GeV] // energy deposited on the hit
      - float eDepError [GeV] // error measured on eDep
      - edm4hep::Vector3d position [mm] // hit position
    Types:
      - extension::TrackerHitPlane
      - extension::TrackerHit3D
      - extension::DriftChamberDigi
tmadlener commented 6 months ago

I can't reproduce the exact error you see (i.e. missing include). However, there is indeed a somewhat conceptual issue here. The main issue is that types that are used in interface declare that interface as friend, such that the interface can access its internals.

Since we can't change the headers of the upstream models, we will have to find another solution to this.

m-fila commented 6 months ago

I think it would be another problem if a model and upstream used different getSyntax

tmadlener commented 6 months ago

That is true. However, I would say that that is something that needs to be documented, and I think that it would be a very niche use case to mix datamodels with different getSyntax values.

m-fila commented 6 days ago

I can't reproduce the exact error you see (i.e. missing include). However, there is indeed a somewhat conceptual issue here. The main issue is that types that are used in interface declare that interface as friend, such that the interface can access its internals.

Did I miss something or the interfaces need access to object internals only for operator< that is done by comparing internal pointers?

tmadlener commented 6 days ago

At least initially, they also needed it for setReferences. But that might have changed with https://github.com/AIDASoft/podio/pull/673. I haven't actually checked, whether that changes something.

m-fila commented 6 days ago

Thanks, I'll take a look and maybe propose something