key4hep / eede

EDM4hep Event Data Explorer
https://key4hep.github.io/eede/
BSD 2-Clause "Simplified" License
3 stars 4 forks source link

Load particles according to edm4hep #36

Closed brauliorivas closed 2 months ago

brauliorivas commented 3 months ago

BEGINRELEASENOTES

ENDRELEASENOTES

The idea here is to follow the goal mentioned by @tmadlener, to always show the latest version. The output/datatypes.js file will be used to know which members and relations (name + type) has each type of particle. Then, when loading an event, the data will be shown according to the file. However, two things may happen:

Then, we could have a "general" particle for each type, that will access members and relations, to later have some methods that will allow to draw that particle, or draw a relation towards a type of particle.

brauliorivas commented 3 months ago

I suppose this is a somewhat parallel development for #25 with a slightly more automated approach to generating the loading functionality, right?

Indeed, this way we get the latest version.

I am not yet sure it will allow us to read also older files that have been written with an older version of EDM4hep

I'm not either. The option is to try to read the common things between versions, but if some particle changes too much, it isn't very helpful. We could (a bit "ugly solution") include (all, some?) versions of edm4hep when generating the file with definitions. This way, we can use the latest version by default, or the one that best fits an object from edm4hep when reading a JSON file. We may also try to fetch the version according to releases but this would require a server (unless the user wants to clone the repo locally and run some sort of script). Another way is to simply read the common members/relations across versions. So for example if the latest version of MCParticle had members a and b, but the user has a file with an MCParticle from an old version with members a and c, then we only read a, b is blank and c is ignored. I think the first solution is better even though is not so orthodox, it serves well.

github-actions[bot] commented 3 months ago

PR Preview Action v1.4.7 :---: Preview removed because the pull request was closed. 2024-06-19 14:34 UTC

brauliorivas commented 3 months ago

One thing that I don't yet understand is: When is the generation of the datatypes.js actually run? I assume it's done whenever necessary offline? Is this manually triggered? If that is the case, could you put just a few lines / commands into a markdown file at least minimally describing the process?

Yes, it can be done offline and later push changes to dmx. However, we could create a workflow (trigger under click, or when a new release from edm4hep). Anyway, I'll add a markdown file for this process.

I have also left a few smaller comments. The main one I have at this point is about naming. There is still quite a bit about particle in function and variable names. However, IIUC, this could all be object or type (depending on context), as it applies to all datatypes not just particle like ones, right?

Yes, it can be applied to anything.

Finally (also in one of the inline comments), it should be possible to also fit MCParticle into this scheme, right? Could you do that, just to see if it is compatible with the current approach? If not, you might have to adapt either this dynamic loading or the current implementation for the MCParticle, because in that is no special datatype, it was just the one we started with.

Right! I'll add MCParticle to the schema.

brauliorivas commented 3 months ago

I am not sure if it is already the case, but if it isnt: If we can make this PR to load the MCParticles that we display at the moment, I think we could go forward with this and then start to work on displaying more types in the next one.

You are right, that way we can check how this is going.

brauliorivas commented 3 months ago

This looks good to me. One thing that we could add as a test is to add a small JSON file that we can load and then make some assertions on to make sure that things work with "real" inputs and that the links are to the correct objects.

Sure, I'll add this 👍🏻.

brauliorivas commented 2 months ago

@kjvbrt anything from your side for this? Otherwise, I would propose to merge this and then starting to work on the visualizations on top of this.

Yeah, please 😃

tmadlener commented 2 months ago

One final thought of mine. This is able to handle multiple collections of the same type, right? For MCParticle there will usually only be one, but e.g. for Tracks or ReconstructedParticle there might be multiple collections. We can always address that in a separate PR if not yet.

brauliorivas commented 2 months ago

One final thought of mine. This is able to handle multiple collections of the same type, right? For MCParticle there will usually only be one, but e.g. for Tracks or ReconstructedParticle there might be multiple collections. We can always address that in a separate PR if not yet.

Yes, on a single event could exist multiple collections of the same type. So here all the collections are loaded and then "merged".

tmadlener commented 2 months ago

Is it then still possible to only view selected collections?

brauliorivas commented 2 months ago

Is it then still possible to only view selected collections?

Yes. But I think this should be developed when working on visualization.

kjvbrt commented 2 months ago

This looks good to me, let's continue with the visualizations... :)