Skip to content

feat: implement new optimal control example #124

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

Merged
merged 13 commits into from
Apr 11, 2025

Conversation

YousefElbrolosy
Copy link
Contributor

I have created a new folder under tutorials-v5 called quantum optimal control, to include examples of how to apply optimal control algorithms, since many of the linked examples to qutip-notebooks (https://qutip.org/qutip-tutorials/) are now outdated.

This is an updated implementation using qutip_qtrl based on the deprecated notebook of GRAPE CNOT implementation by Robert Johansson

Was directed here by @hodgestar after a pull request to fix an outdated notebook in https://github.com/qutip/qutip-notebooks

@pmenczel
Copy link
Member

pmenczel commented Apr 7, 2025

Hi and thank you for contributing. Migrating optimal control notebooks to qutip-tutorials and updating them is very good work!

Please have a look at the main page of this repository. This repository does not contain tutorials in ipynb format, but rather in jupytext markdown format. In markdown format, it will be easier to review the contents of the notebook.

@rochisha0 is also migrating optimal control notebooks in PR #122. However, this one here still seems to be missing from the other PR, so that is good. Could you please have a look at the other PR and follow the folder structure there?

@YousefElbrolosy
Copy link
Contributor Author

Done!

@pmenczel
Copy link
Member

pmenczel commented Apr 9, 2025

Thank you! You are adding the same file twice now I think, the "quantum optimal control" one should be deleted.

It seems like the drift Hamiltonian was zero in the old notebook, and is not zero any more here. Could you just briefly explain the changes?

@YousefElbrolosy
Copy link
Contributor Author

Thank you! You are adding the same file twice now I think, the "quantum optimal control" one should be deleted.

It seems like the drift Hamiltonian was zero in the old notebook, and is not zero any more here. Could you just briefly explain the changes?

You are correct, I was experimenting with it and forgot to change it back.
I fixed it and removed the extra file.

Copy link
Member

@pmenczel pmenczel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I am adding a few suggestions for improvement.

@ajgpitch I see that the other control notebooks that @rochisha0 has converted are numbered now. How did you imagine this, should we just give this one the number 08?

@YousefElbrolosy
Copy link
Contributor Author

YousefElbrolosy commented Apr 10, 2025

Thank you. I am adding a few suggestions for improvement.

@ajgpitch I see that the other control notebooks that @rochisha0 has converted are numbered now. How did you imagine this, should we just give this one the number 08?

I have incorporated your comments, let me know.

Also, maybe, when @rochisha0 merge passes the build tests, I can rebase and then modify the rest of the numbering so that all the grape methods are numbered one after the other to maintain natural ordering.

@YousefElbrolosy
Copy link
Contributor Author

YousefElbrolosy commented Apr 11, 2025

Any ideas, about fixing this dead links error in the CI/CD checks?

@pmenczel
Copy link
Member

Any ideas, about fixing this dead links error in the CI/CD checks?

It's a problem that was fixed in another PR... could you merge the current main branch into this one? Then the tests should pass (and if they do, I'll merge :))

@YousefElbrolosy YousefElbrolosy force-pushed the implement-grape-cnot-tutorial branch from 90ad17f to 20cd146 Compare April 11, 2025 12:43
@YousefElbrolosy
Copy link
Contributor Author

Any ideas, about fixing this dead links error in the CI/CD checks?

It's a problem that was fixed in another PR... could you merge the current main branch into this one? Then the tests should pass (and if they do, I'll merge :))

I did, and modified the notebook name to be consistent as well could you please try the workflow?

@pmenczel
Copy link
Member

Great, thank you! Reminder @ajgpitch and @rochisha0 , feel free to rename this to fit your naming scheme and add it to the overview when you add the overview back.

@pmenczel pmenczel merged commit 6531566 into qutip:main Apr 11, 2025
3 checks passed
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.

2 participants