Skip to content

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?

is_state_transfer = False
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.

@ajgpitch
Copy link
Member

Okay, merging this now, despite concerns that this does not actually get tested on the assumption that #43 and #44 will be merged soon and will test this.

@ajgpitch ajgpitch merged commit ee0381f into qutip:main Aug 15, 2025
12 checks passed
@pmenczel
Copy link
Member

@ajgpitch Thanks for merging, I will go over those notebooks in #44 one more time and hope it will be ready for merging then.

Just to clarify though: this PR fixed the "symptom" but not the underlying problem of the final state being computed in multiple different ways, right?

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
4 participants