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

Merged
merged 16 commits into from
Dec 2, 2024

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.

Add the new category so that it can be read
Effectively a header per link with a typedef and a common .cc file with
registration code for all links
Otherwise things start to break in cases where users explicitly use
these types at the moment.
doc/links.md Outdated Show resolved Hide resolved
python/podio_gen/test_ClassDefinitionValidator.py Outdated Show resolved Hide resolved
@tmadlener tmadlener merged commit 39587ee into AIDASoft:master Dec 2, 2024
18 checks passed
@tmadlener tmadlener deleted the links-in-yaml branch December 2, 2024 10:10
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