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

Apply default_dtype in Qobj and operation. #2519

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Ericgig
Copy link
Member

@Ericgig Ericgig commented Aug 27, 2024

Description
Add a new options that control where the default_dtype is taken into account: default_dtype_range (feel free to propose better name.)
It has 3 possible values:

  • "creation": At Qobj creation functions, behaviour we presently have.
  • "missing": Missing specialization will output that type: Dense + Jax is not defined so will default to that type.
  • "full": Every operations return that type: Dense + Dense would convert the result to the default_dtype. Unary operations will also convert the output type. It could break some functions that explicitly set the date type such as steadystate or HEOM...

Also added control of the created format in Qobj.__init__: added dtype, when the initial data is a list of list, use default_dtype, with "full" also use default_dtype. Only using the default value with list normally is so solver states are not converted automatically which would slow the solver unexpectedly.

I am looking into creating alias for a group of data layer. This would allow to set the default to jax and jaxdiag at once. Used with "full", this would ensure only jax compatible data layers are used while not forcing the sparcity. This would hopefully make "full" usable.

Related issues or PRs
#2328

@coveralls
Copy link

coveralls commented Aug 27, 2024

Coverage Status

coverage: 87.741% (+0.01%) from 87.729%
when pulling 52264ce on Ericgig:feature.dtype_range
into eb3e5c4 on qutip:master.

@Ericgig
Copy link
Member Author

Ericgig commented Oct 8, 2024

@nwlambert
I tried to address the issue of defautl dtype not being apply where expected here.
It probably won't affect the brmesolve issue but I would like you opinion on the approach.

@nwlambert
Copy link
Member

thanks eric, I like the option of creation and full. Took me a moment to understand the meaning of the ''missing'' option. is there some clear situation where someone would use this? perhaps there is a better name too?

for default_dtype_range, how about default_dtype_scope?

i will try and play with the PR, see if it does what i would expect

@Ericgig
Copy link
Member Author

Ericgig commented Oct 11, 2024

I don't think either "missing" or "full" should be used much with pure qutip. Only with plugins like qutip-jax it makes any sense to me. With "missing", you could set it so it will end up all in jax format and not have too much issues.

"full" will probably break HEOM, some steadystate feature, slow down scipy integrators, etc. "missing? should be safe.

Ok for default_dtype_scope.

@nwlambert
Copy link
Member

I had a quick play with the feature, for the cases I had stumbled on before, with `eigenstates()' ignoring default_dtype, using the scope of 'full' covers it now. I understand having this work with just 'creation' scope is problematic because you want to make sure the dtype is not used on results lists.

heom+full breaks with dia and dense, as expected!

speaking of heom, manually converting the RHS to jaxdia and doing stuff it worked fine. i think generalizing the RHS construction in heom away from csr is probably very difficult without huge performance drop in the construction, so perhaps some kind of wrapper or option to do conversion of types after construction would be fine?

@Ericgig
Copy link
Member Author

Ericgig commented Jan 29, 2025

I tried running tests with scope=full and it has priority over manually entered dtype values...
Not that using object in those other type is really possible with that options, it still not great.

@hodgestar
Copy link
Contributor

For the HEOM, perhaps the HEOM solver could gain an option to select the dtype to convert the RHS to?

@Ericgig Ericgig requested a review from nwlambert February 4, 2025 06:47
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.

4 participants