Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a new links category for the YAML definitions #691

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

tmadlener
Copy link
Collaborator

@tmadlener tmadlener commented Sep 26, 2024

BEGINRELEASENOTES

  • Introduce a new links category into the YAML grammar to automate the declaration of Links.

ENDRELEASENOTES

Diff wrt to #257

As can be seen from the diff above, I needed to change the definition of the PODIO_DECLARE_LINK macro, and it now only deals with the registration to the CollectionBufferFactory and there is a separate macro to deal with the registration to the SIOBlockFactory. Otherwise, it becomes hard to only build the "central" part of a datamodel without any dependencies on an I/O backend. This could be lifted to #257 if we wanted.

@tmadlener
Copy link
Collaborator Author

From a technical point of view this is ready. I can successfully build the complete stack with this and key4hep/EDM4hep#373 without any changes to the stack, apart from a few cases where the deprecated Association headers were still in use and which should be fixed soon.

doc/datamodel_syntax.md Outdated Show resolved Hide resolved
@tmadlener
Copy link
Collaborator Author

@peremato this should also address your concerns about having to discover these links for the Julia interface, I think. Now they will just be listed as a separate category in the YAML file, and you should have all the necessary information again to generate the necessary code, I think.

doc/links.md Outdated Show resolved Hide resolved
doc/datamodel_syntax.md Outdated Show resolved Hide resolved
python/podio_gen/cpp_generator.py Outdated Show resolved Hide resolved
python/podio_gen/julia_generator.py Outdated Show resolved Hide resolved
@tmadlener
Copy link
Collaborator Author

Now the layout in the files also uses a vector<podio::LinkData> instead of a vector<float> to store the weights of the links. This should make the switch from the pre-templated links to these ones completely transparent (apart from the changed typename).

@tmadlener
Copy link
Collaborator Author

Unless there are any more review comments, I will merge this later this week, to let EDM4hep move forward towards its 1.0 release.

@hegner
Copy link
Collaborator

hegner commented Nov 20, 2024

Absolutely fine with me. If you want I can make a full review of the PR now

@hegner hegner self-requested a review November 20, 2024 13:25
@tmadlener
Copy link
Collaborator Author

I am also waiting for a final confirmation from @peremato to make sure that the files that will be produced with this version are as expected on the Julia side.

tmadlener and others added 7 commits November 20, 2024 20:48
Co-authored-by: Mateusz Jakub Fila <37295697+m-fila@users.noreply.github.com>
Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>
Otherwise ROOT does something clever and simply stores the float without
the struct. Which makes the switch over a bit less transparent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants