Fix state transfer not working for GRAPE and CRAB #36
Conversation
Pull Request Test Coverage Report for Build 13915520915Details
💛 - Coveralls |
|
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? |
|
@pmenczel I'll check again. |
|
@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. |
|
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) |
src/qutip_qoc/pulse_optim.py
Outdated
| 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": |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
I have refactored in a different way. Can you please check that, or explain this way more?
| objective.target = qt.to_super(objective.target) | ||
| elif optimization_type is None: | ||
| if qt.isoper(objective.initial) and qt.isoper(objective.target): | ||
| if np.isclose(qt.tr(objective.initial), 1) and np.isclose(qt.tr(objective.target), 1): |
There was a problem hiding this comment.
This doesn't work because qt.tr doesn't exist. (I am confused that the tests pass, they really do not seem to be testing much.)
Otherwise it looks good to me, but when you have fixed this could you please check that everything works by trying to run some of the example notebooks?
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