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

Triage torch_tensor_from_array #58

Closed
4 tasks
jatkinson1000 opened this issue Nov 1, 2023 · 1 comment · Fixed by #60
Closed
4 tasks

Triage torch_tensor_from_array #58

jatkinson1000 opened this issue Nov 1, 2023 · 1 comment · Fixed by #60

Comments

@jatkinson1000
Copy link
Member

We need to decide if it is best to:

  • require users to specify the dtype when calling torch_tensor_to_array()
  • Stick with the abstract interface approach currently there.

If the latter then interface needs expanding beyond the current 'float' and 'double'.

I can see an argument for the former, however, as it

  • a) simplifies the code (one routine and no interface), and
  • b) forces the user to be explicit about the data type they are using which, as we have seen, causes issues if incorrect.

Discussion appreciated, then we need to reach a decision and triage:

  • Amend source as required
  • Update docs as required
  • Update examples as required
  • Update benchmarks as required
@jatkinson1000
Copy link
Member Author

Discussion:

  • @TomMelt Thinks the interface for dtype is a good idea - keep this and expand the types interface in a PR.

Discussion that torch_tensor_from_array is perhaps not best as a simplified torch_tensor_from_blob:

  • Instead should it perhaps also abstract array the shapes from the user and be written to accept Fortran objects
    we can then infer shapes, strides, rank etc.
  • We could add F of C as an optional argument Simplify stride interface with F or C layout #56
    However, consider making this a required argument. Or take it out completely.
  • For anything more complicated than this we point users to using torch_tensor_from_blob

Handling Different shapes and arrays:

  • To abstract away the inferred shape and dimensions we would need to add an interface for each possible dimensionality
  • This could get tedious, but we could solve this using fypp (adds a dependency).

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 a pull request may close this issue.

1 participant