-
Notifications
You must be signed in to change notification settings - Fork 109
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
Broadcasting for semiclassical objects #404
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge after a small import change.
And after the errors in the tests are fixed. Does this depend on the changes to QuantumOpticsBase in its broadcast PR? If so, we should bump the compat bound for it. |
And same comment as in the QuantumOpticsBase PR on adding your examples from the PR description to the test runner, maybe as a new file. |
Yes, so we should merge this after we merge the QuantumOpticsBase PR. |
@Krastanov Should be good to go. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #404 +/- ##
==========================================
- Coverage 97.03% 96.66% -0.38%
==========================================
Files 18 19 +1
Lines 1554 1678 +124
==========================================
+ Hits 1508 1622 +114
- Misses 46 56 +10 ☔ View full report in Codecov by Sentry. |
@Krastanov is there anything preventing this from getting merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
There is a minor issue with isapprox
not propagating its kwargs.
More importantly though, there are a few functions that codecov reports as not covered by tests. Could you add new tests for them, just to ensure the review is not missing anything? I am a bit skittish here as this touches a lot of basic interfaces.
@Krastanov good catch. Should be covered now. |
@david-pl @Krastanov @amilsted
State
objects, which enables use of DiffEq solversState
s can be naturally written as the following example:State
is weird, and really the only reason we're doing it is to have interoperability of semiclassical objects with SciML.State
objects were recasted in thesemiclassical.schroedinger_dynamic
,semiclassical.master_dynamic
, etc. functions.