-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add missing Time Evolution Tutorials #31
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.
I ended up doing a reasonably thorough review because I got interested in the optomechanical example after this morning's discussion. :)
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Co-authored-by: Simon Cross <[email protected]>
Thanks for the review! :) I updated the tutorial accordingly. I also reduced the number of solvers used in |
I have moved the tutorials into the |
# use QuTiP's builtin parallelized for loop, parfor | ||
results = parfor(brme_step, itertools.product(wd_list, Om_list)) | ||
# use QuTiP's builtin parallelized for loop: parallel_map | ||
results = parallel_map(brme_step, list(itertools.product(wd_list, Om_list))) |
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.
It would be nice if this example should be using the class interface instead of the function call in v5.
The class interface is to be used when computing the same system with different parameters, which is what is done here.
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.
Thanks for the review @Ericgig! I changed from using brmesolve
to BRSolver
in the last commit. BRSolver
currently requires that the spectra for the a_ops
are given as coefficient
. Hence, I can not pass a string spectrum.
I think it would be great if BRSolver
also accepts other types of spectra.
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.
Also looks good. Some questions:
- Are notebooks 15, 16 and 17 in a separate PR?
- It would be nice to automatically trim empty python cells at the end of the notebooks, or at least to warn about them in the test runs, but maybe just create an issue for that?
tutorials-v5/time-evolution/010_brmesolve_phonon_interaction.md
Outdated
Show resolved
Hide resolved
Thanks for the review, @hodgestar! I opened an issue to create a test for empty code cells in #38. |
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.
It looks like some images here are still in a top-level images folder, but once they're moved closer to their notebooks I'm happy for this to be merged.
This adds more tutorials on steady state and stochastic solver.