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

Fix state transfer not working for GRAPE and CRAB #36

Merged
merged 5 commits into from
Mar 28, 2025

Conversation

rochisha0
Copy link
Collaborator

@rochisha0 rochisha0 commented Feb 26, 2025

This PR adds check when Hamiltonian is operator and passes dyn_type as UNIT for state transfer
Also fixes the checks for open systems
Fixes #34

@coveralls
Copy link

coveralls commented Feb 26, 2025

Pull Request Test Coverage Report for Build 14114330842

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 14 of 31 (45.16%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.4%) to 86.59%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/qutip_qoc/pulse_optim.py 14 31 45.16%
Totals Coverage Status
Change from base Build 13404420638: -1.4%
Covered Lines: 833
Relevant Lines: 962

💛 - Coveralls

@pmenczel
Copy link
Member

Thank you Rochisha,

I just ran the notebook in #34 again, and the problem still seems to be the same. Am I doing something wrong?

@rochisha0
Copy link
Collaborator Author

@pmenczel I'll check again.

@pmenczel
Copy link
Member

@rochisha0 I am very sorry, I was using the wrong conda environment. It seems to be working, I will give it a closer look when I have time.

@nwlambert
Copy link
Member

just a note, lets chat a bit later today before merging this, I am a bit worried it conflicts with the operator->operator control examples (in the sense of optimizing unitaries/propagators)

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.

Argh, sorry, Neill is right.

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 for the update, I have just a few QOL things to add

Comment on lines 197 to 199
for objective in objectives:
if any(qt.issuper(H_i) for H_i in (objective.H if isinstance(objective.H, list) else [objective.H])):
if optimization_type == "state_transfer":
Copy link
Member

Choose a reason for hiding this comment

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

To make everything a bit easier to read and maintain, could we refactor a little bit so that this whole block of code becomes, for example

for objective in objectives:
    if objective._is_open():
        objective._format_for_open(optimization_type)

Without changing any of the logic; the logic seems good to me :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have refactored in a different way. Can you please check that, or explain this way more?

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, it seems to be working now in all notebooks that I tested. I will merge the PR.

@rochisha0 @ajgpitch This means that once the QTRL-release is done, QOC is ready to make a release as well.

@pmenczel pmenczel merged commit 7981bb5 into qutip:main Mar 28, 2025
12 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.

State transfer not working with GRAPE and CRAB, and with open systems
4 participants