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

Exposing hidden conversions between data types in qutip-tensorflow. #35

Open
AGaliciaMartinez opened this issue Dec 7, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@AGaliciaMartinez
Copy link
Member

Description

One of features that QuTiP brings with the new data layer is the possibility to automatically convert between data layers to perform operations for which an specialisation does not exist. This is useful in some cases as it means you will not need to define every specialisation once a new data type has been added. However, this may pose some problems for qutip-tensorflow as we do not want automatic conversion of data types to happen, for example, when using the following non-defined specialisation:
add(TfTensor128, TfTenso64)
I am no sure whether TfTensor128 will be downcasted to TfTensor64 or the other way around. In any case, if such operation is being performed I would prefer a warning being raised. Similarly, converting from Dense to TfTensor is okay but converting TfTensor to Dense is probably not desired in most (if not all) cases. This is because it can happen automatically and not be noticed while using the automatic differentiation feature.

I anticipate that in most cases this issue will affect the user because the user forgot to convert one of the Qobj to the appropriate data type. Raising an error in some of these cases would be ideal to help debugging.

Note, that we still want to use the to method when used by the user explicitly.

Possible solution 1 - not ideal

We can create an specialisation for the cases were no conversion is wanted, add(TfTensor128, TfTensor64), add(Dense, TfTensor64) for example. This specialisation will raise a TypeError exception. However, this means that new specialisations added by other packages will still be converted, so it is not ideal.

Possible solution 2

Instead of adding a new specialisation, we can make the conversion specialisations (_tf_to_dense in this case) to raise a custom warning: NotASafeConversion. This conversion would then be caught by the to method in Qobj and be ignored assuming that the to method for Qobj is used only if the user is sure of what it is doing.

I would ideally like to raise an exception and not a warning but I am not sure how to do this as raising an exception in the conversion specialisation would also break the to method at the Qobj level. Maybe modifying the dispatcher allow this to work?

@AGaliciaMartinez AGaliciaMartinez pinned this issue Dec 7, 2021
@AGaliciaMartinez AGaliciaMartinez unpinned this issue Dec 7, 2021
@AGaliciaMartinez AGaliciaMartinez pinned this issue Dec 7, 2021
@AGaliciaMartinez AGaliciaMartinez unpinned this issue Dec 7, 2021
@AGaliciaMartinez
Copy link
Member Author

@jakelishman, @Ericgig I remember something like this being mentioned in one of the GSoC meetings we had by one of you. What are your thoughts on this?

@hodgestar
Copy link
Contributor

Some additional trickiness:

  • .to(...) might be called explicitly in a variety of places (e.g. inside solvers if an integrator needs a particular data format, etc)
  • Even if we added a .to(..., implicit=True) flag, the same conversion might be implicit or explicit depending on the point of view of the user (e.g. user A might say "I explicitly specified solver X which I know requires CSR matrices" while user B might say "I specified solver X, but I didn't want the data type to change").

Probably we need more explicit control over which data layer types are in use rather than a narrow solution to this particular case.

Jake had an idea for data type implementations to not be activated automatically when they're registered. That might provide a great solution to this and other cases while keeping the number of active data types manageable and allow users to explicitly enable them with, e.g., qutip.data.enable("tftensor64") (possibly as a context manager).

There's a lot of rope here for us to make things messy with, so we should think carefully.

@AGaliciaMartinez AGaliciaMartinez added the enhancement New feature or request label Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants