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 DType class or so? #137

Open
albertz opened this issue Apr 28, 2022 · 1 comment
Open

Introduce DType class or so? #137

albertz opened this issue Apr 28, 2022 · 1 comment
Milestone

Comments

@albertz
Copy link
Member

albertz commented Apr 28, 2022

Currently all dtype arguments are simply str.

This works ok but might be a bit problematic whenever we want to check the dtype in a more custom way. E.g. currently you find sometimes code like if "int" in dtype or so, which is maybe slightly wrong and not nice. Also obviously there is no auto-completion for this but this is maybe minor.

What are the options:

  • Introduce our own DType class and all the standard types. Basically replicate what TF/Numpy/PyTorch/etc has.
  • Reuse TF or Numpy dtypes.
    • TF disadvantage: Maybe we want to switch at some point to another framework, so I would avoid having explicit TF dependencies which cannot easily be replaced.
    • Numpy disadvantage: I think it does not perfectly covers what we support.
  • Just leave it as it is, i.e. only accept str.

Orthogonal:

  • Require a real DType everywhere, or also allow str? This however makes all internal code more complicated, always requiring sth like nn.as_dtype(...).
@Atticus1806
Copy link
Contributor

I like the idea of introducing them. Maybe for now we can start with simple mappings to strings and extend that later when needed? Also if code like: nn.int32 is available I dont see why we would allow strings then. If the user wants to use strings, we can either provide a str->dtype function or leave it for the user to map.

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

No branches or pull requests

2 participants