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

Document and test input_dtype and output_dtype policy #350

Open
Michael-T-McCann opened this issue Oct 5, 2022 · 1 comment
Open

Document and test input_dtype and output_dtype policy #350

Michael-T-McCann opened this issue Oct 5, 2022 · 1 comment
Labels
discussion required Some discussion necessary to decide how to address this issue documentation Improvements or additions to documentation tests Pertaining to SCICO tests

Comments

@Michael-T-McCann
Copy link
Contributor

Michael-T-McCann commented Oct 5, 2022

We should decide what input_dtype and output_dtype mean, then document it.

Proposed input_dtype and output_dtype policy:

  • All instantiated Operators must have input_dtype and output_dtype properties
    • Possible data types are snp.float32, snp.float64, snp.complex64
  • at the level of Operator, we should call snp.dtype(input_dtype) to make sure it is a dtype and not a type
    • compare print(snp.float32) and print(snp.dtype(snp.float32)), second has nice repl
  • For an Operator H, H(x) should throw an error if x.dtype is not input_dtype
    • Instead: how about input_dtype is the "largest" dtype allowed, passing a "smaller" one is allowed
      • Would this still work on the adjoint? NO, so largest dtype does not work
  • Operators should be written so that H(x).dtype is H.output_dtype; this will not be checked and runtime, but should be tested
  • Operators should attempt to automatically deduce input_dtype and output_dtype from other arguments and throw an error if the user requests an input_dtype or output_dtype that is not realizable

This policy should be described in the docs and implemented in the code.

Related: #165 #234

@Michael-T-McCann Michael-T-McCann added the discussion required Some discussion necessary to decide how to address this issue label Oct 5, 2022
@bwohlberg bwohlberg added documentation Improvements or additions to documentation tests Pertaining to SCICO tests labels May 8, 2023
@bwohlberg bwohlberg mentioned this issue Dec 9, 2023
@Michael-T-McCann
Copy link
Contributor Author

Thoughts after more discussion:

  • We want dtypes to be as unobtrusive on the user as possible because they often won't matter (everything will be float32). But it would be good to be able to correctly deal with float64 (and float16 etc.?) when desired.
  • It's not clear we even need dtype properties at all.
  • dtype also currently encodes whether the output/input are real/complex, but these could be separated.
  • Could dtype for linear operators by analogous to dtype for narrays? But then how to deal with dtype for nonlinear operators?
  • one technical problem may be that linear_transpose wants to know dtypes and is currently called at init. Maybe we can get around that by memoizing and calling linear_transpose only when adj is called.

Next steps:

  • look at examples to see how (if) input/output_dtype is used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion required Some discussion necessary to decide how to address this issue documentation Improvements or additions to documentation tests Pertaining to SCICO tests
Projects
None yet
Development

No branches or pull requests

2 participants