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

Convert event parameters from LCIO to EDM4hep #184

Merged
merged 1 commit into from
May 24, 2024

Conversation

Zehvogel
Copy link
Contributor

@Zehvogel Zehvogel commented May 7, 2024

BEGINRELEASENOTES

  • Add lcio to edm4hep event parameter conversion

ENDRELEASENOTES

@tmadlener
Copy link
Contributor

Technically, I think the const_cast should be OK in this case as the original Frame is not const. Additionally, the underlying putParameter (and the GenericParameters) are thread safe (via locking). From that point of view the PodioDataSvc could in principle also hand out a mutable Frame via getEventFrame.

@Zehvogel
Copy link
Contributor Author

Zehvogel commented May 8, 2024

Yes I mainly meant it has to go as I would consider it ugly...

I am in favour of returning a non-const frame to get rid of this quickly. :)

I am not so sure about the sustainability in the long run though. The whole being able to access the frame directly via the datasvc from any Algorithm leads the whole concept of Algorithms having to specify in-/output collections ad absurdum, doesn't it?

@tmadlener
Copy link
Contributor

I am not so sure about the sustainability in the long run though. The whole being able to access the frame directly via the datasvc from any Algorithm leads the whole concept of Algorithms having to specify in-/output collections ad absurdum, doesn't it?

Yes, fully agree. I am not sure if there is an easier way at the moment. In principle I think only the MarlinWrapper currently needs these special accesses. But that will probably also be around for quite some time.

Zehvogel added a commit to Zehvogel/k4FWCore that referenced this pull request May 10, 2024
See discussion in: key4hep/k4MarlinWrapper#184

Should be cleaned up in the future.
Zehvogel added a commit to Zehvogel/k4FWCore that referenced this pull request May 10, 2024
See discussion in: key4hep/k4MarlinWrapper#184

Should be cleaned up in the future.
@Zehvogel Zehvogel force-pushed the parameter-conversion branch from 59e5c67 to ed0a94d Compare May 13, 2024 12:16
@Zehvogel Zehvogel marked this pull request as ready for review May 17, 2024 15:07
@Zehvogel
Copy link
Contributor Author

can someone re-trigger the ci pls? :)

@andresailer
Copy link
Collaborator

Can you not?

@Zehvogel
Copy link
Contributor Author

Can you not?

No, and I think it is not only because I am too blind to find the button...

@andresailer
Copy link
Collaborator

Can you not?

No, and I think it is not only because I am too blind to find the button...

True, https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs

But the jobs still fail.

@Zehvogel
Copy link
Contributor Author

Oh yes, the merge this morning just felt so far away that I thought it was yesterday... it will take another day until the nightlies picked it up :(

@tmadlener tmadlener changed the title event parameter conversion Convert event parameters from LCIO to EDM4hep May 24, 2024
@tmadlener tmadlener merged commit b395c85 into key4hep:main May 24, 2024
5 of 10 checks passed
jmcarcell added a commit that referenced this pull request May 27, 2024
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.

3 participants