Created flimesolve.py #2140
Created flimesolve.py #2140
Conversation
Reconstructed FLiMESolve and FLiMESolver to play more nicely with other QuTiP functions. Wrote FLiMESolver into the correlation tab, but had to add taulist as an input into the _make_solver function.
Trying to rework the time list stuff in FlimeSolve/er to work better with QuTiP. Namely, autogeneration of the time list to create the rate matrix & trying to avoid time list in solver.run so I don't have to do anything kludgey to get the modes in an expedient way
Rewrote the mode table in solver.run() to be found in a slightly slower, but much more generalized fashion
Figured out how to do Expectation operators with my construction of the solvers, and I added a constraint the to the available frequencies in the relaxed secular approx. based on the Nyquist theorem.
Floquet beta
Added flimesolve to dynamics-floquet documentation, and added floquet_ex4 as an example usage (identical to floquet_ex3 with exception of use of flimesolve instead of fmmesolve) Added towncrier changelog...I think
…to qutip-5.0.X
Got rid of a few commented lines that were not useful
Deleted folder of scripts I'm using to write a paper about this solver
There was a problem hiding this comment.
Something is strange in floquet.py, did you include change not meant to be here?
Both floquet.py and flimesolve.py have the original fmesolve and the new flimesolve...
Tests should pass.
Right now, scipy 1.11 breaks cvxpy which we use so this breaks a few tests, but the changes you made to correlation.py also break the tests.
We will want tests for the new solver.
Could you also review the style (pep8).
- There are some very long line, we use the official 80 characters width.
- space are not uniform, sometime they are doubled, other time missing.
- There are rules for empty lines, there should not be 3 empty lines in a function.
|
Edit: I thought that the original pull request would have been killed such that I would need to submit a new one. I don't think this is the case, as of viewing still open request on Git, so I wont be submitting another pull request unless otherwise informed.
I'm going to submit another pull request, and it should be much better this
time. I made these changes:
- Fixed whitespace, blank line, and linting issues as far as I can tell
- I'm not sure that I broke lines up entirely the best way in all
cases, but I think it looks okay enough as of now.
- Took all of my new functions out of floquet.py and placed them
properly into flimesolve.py
- The version of floquet.py that I'll be submitting was one I
directly copy-pasted from the main repository, such that there
shouldn't be any changes whatsoever to it
- replaced all scipy functions with numpy equivalents
- Fixed the issues in correlation.py, as far as I know
- Turns out the issue here was that I never updated correlation.py to
my most current version, which should only add in flimesolve as an option
for solvers (with an if statement or two to get the Hamiltonian into a
FloquetBasis object if flimesolve is the selected solver).
Thanks again for your feedback!
…On Mon, Jun 26, 2023 at 6:32 PM Fenton Clawson ***@***.***> wrote:
Hey Eric,
Looks like I did mess up with the floquet.py versus flimesolve.py. I think
I forgot at some point that I was to include all my new stuff in just the
flimesolve.py, and then forgot about that script altogether. I apologize
for this, and I'll fix it.
I'll also go back through and fix style issues. As with above, I think I
forgot to keep the proper styling at a certain point.
I tried to keep my changes to correlation.py to a minimum, and contained
to simply adding in the new solver in the make_solver function, as well as
adding in some inputs as options to that solver, but I'll go through and
review to see what's wrong.
Thanks for your feedback, and I'll try to get this all fixed up as soon as
I can.
On Mon, Jun 26, 2023, 4:49 PM Eric Giguère ***@***.***>
wrote:
> ***@***.**** requested changes on this pull request.
>
> Something is strange in floquet.py, did you include change not meant to
> be here?
> Both floquet.py and flimesolve.py have the original fmesolve and the new
> flimesolve...
>
> Tests should pass.
> Right now, scipy 1.11 breaks cvxpy which we use so this breaks a few
> tests, but the changes you made to correlation.py also break the tests.
> We will want tests for the new solver.
>
> Could you also review the style (pep8).
>
> - There are some very long line, we use the official 80 characters
> width.
> - space are not uniform, sometime they are doubled, other time
> missing.
> - There are rules for empty lines, there should not be 3 empty lines
> in a function.
>
> —
> Reply to this email directly, view it on GitHub
> <#2140 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AV5WL5EA3UU23BHDUONIEN3XNHYVNANCNFSM6AAAAAAWLGP4AU>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
--
Best Regards,
Fenton Clawson
|
I fixed linting, extra spacing, and blank line issues. Moved all new functions to flimesolve.py. Replaced scipy functions with Numpy equivalents. Fixed issues in correlation.py by updating to my most current version, which should just introduce flimesolve.py as an additional solver.
Commented out a [0] in line 91 for some reason. I uncommented it now to bring it better in line with correlation.py
Fixing code format
There was a problem hiding this comment.
I took a first look.
I see a lot of small style issues, I would suggest that you try a formatting tool: pycodestyle, flake8, black, etc.
I will go into detail on flimesove.py later, but would you mind if I did a pull request agins your local branch to make the tensor using qutip's types. We want to force the use of numpy to the minimum so it can work with other backend, (jax, cupy, tensorflow, etc.).
|
I changed the merge target from |
Co-authored-by: Eric Giguère <eric.giguere@calculquebec.ca>
Co-authored-by: Eric Giguère <eric.giguere@calculquebec.ca>
Co-authored-by: Eric Giguère <eric.giguere@calculquebec.ca>
Co-authored-by: Eric Giguère <eric.giguere@calculquebec.ca>
|
Used the auto-format feature for PEP8 in my IDE (Spyder), then did a second check with pycodestyle and fixed all the issues it raised. Fixed the other issues you raised as well. |
Used built-in autoformat in Spyder and then pycodestyle afterwards to fix code format to PEP8 guidelines.
…to qutip-5.0.X
|
Merged
Merged my qutip-5.0.X to master |
Please feel free to do a pull request. Alternatively, I can spend some time trying to figure it out myself. Not trying to add anything onto your workload. |
Description
Added a new open system solver based on Floquet theory