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

Add edm4hep::Tensor type for use in ML training and inference #388

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

veprbl
Copy link
Contributor

@veprbl veprbl commented Dec 4, 2024

BEGINRELEASENOTES

  • Added edm4hep::Tensor type for use in ML training and inference

ENDRELEASENOTES

This should help to support ML in reconstruction frameworks and to write tensors to disk for training with conventional python ML tools.

@tmadlener
Copy link
Contributor

Hi @veprbl, thanks for this proposal and apologies for the long delay from our side.

We have discussed this proposal in todays EDM4hep meeting and we have a few questions regarding how you (plan to) use this. Presumably, there is already some experience with this from EIC? One of the main concerns that was raised today is that this is obviously extremely generic, and we were wondering whether it is maybe too generic.

  • What are the reasons for choosing float and int64_t as the available datatypes?
  • How do you ensure that shape and the corresponding data remain consistent?
  • Do you store any metadata in some form that describe the content of the tensor? If yes, how are you doing this at the moment?
  • Are you using this for anything else than storing the tensors to disk? Or do you foresee to link this to other datatypes at the moment?
  • Probably a big ask, but could this also work for pytorch?
  • Does ONNX support switching between column / row major layouts? If yes, should this be reflected in the type somehow?

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.

2 participants