Skip to content

Match grape infidelity with manually computed one #51

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rochisha0
Copy link
Collaborator

@rochisha0 rochisha0 commented Jul 22, 2025

Fix #46
With this PR the GRAPE-reported infidelity matches the manually computed one by evolving the system using the optimized control pulses.
key change enable this consistency:

For state transfer problems, the correct fidelity normalization (scale_factor = 1/dim) is applied in the QOC interface, using existing optimization_type checks

@rochisha0 rochisha0 marked this pull request as draft July 22, 2025 09:18
@coveralls
Copy link

coveralls commented Jul 23, 2025

Pull Request Test Coverage Report for Build 16796973140

Details

  • 7 of 14 (50.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.5%) to 86.564%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/qutip_qoc/pulse_optim.py 7 14 50.0%
Files with Coverage Reduction New Missed Lines %
src/qutip_qoc/pulse_optim.py 1 80.69%
Totals Coverage Status
Change from base Build 16036109532: -0.5%
Covered Lines: 844
Relevant Lines: 975

💛 - Coveralls

@rochisha0 rochisha0 marked this pull request as ready for review July 24, 2025 06:35
@rochisha0
Copy link
Collaborator Author

I’ve fixed the issue with looping over the objectives. The problem with using evo_full_final is that it returns the final evolution operator, not the evolved state. This matches what is done in qutip_qtrl/dynamics.py and is probably the reason why it is deprecated.

@ajgpitch
Copy link
Member

The problem with using evo_full_final is that it returns the final evolution operator, not the evolved state

If we are doing state transfer then surely these are the same thing?

@ajgpitch
Copy link
Member

ajgpitch commented Aug 13, 2025

This matches what is done in qutip_qtrl/dynamics.py and is probably the reason why it is deprecated.

What has been deprecated?

if qt.isoper(objective.initial) and qt.isoper(objective.target):
if np.isclose((objective.initial).tr(), 1) and np.isclose((objective.target).tr(), 1):
if np.isclose(objective.initial.tr(), 1) and np.isclose(objective.target.tr(), 1):
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a strange way to check if something is a density matrix

@ajgpitch
Copy link
Member

In no way is a criticism of what @rochisha0 has done to fix the issue, but this all seems like a bit of a mess. Seems strange that we don't ask the user to be more specific about what their objective is rather than us trying to guess based on what they have input, especially if we have no elegant way to distinguish between a density matrix and unitary matrix say. Also, does this code allow for the initial / target to be a CPT map?

I am minded to merge this, as it seems to fix the issue, but open another issue that records the concerns raised above.

@rochisha0
Copy link
Collaborator Author

@ajgpitch , we are considering both state_transfer and gate_synthesis in this case.

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.

Fidelity reported by GRAPE doesn't match manually computed one
3 participants