Defining the new Backend API classes #764
Conversation
| ) | ||
|
|
||
| if default_evaluation_times != "Full": | ||
| eval_times_arr = np.array(default_evaluation_times, dtype=float) |
There was a problem hiding this comment.
why is this a np.array but below you use AbstractArray?
There was a problem hiding this comment.
You mean for interaction_matrix? I use AbstractArray because I expect people might want to differentiate against it
| """ | ||
| pass | ||
|
|
||
| def infer_one_state(self) -> Eigenstate: |
There was a problem hiding this comment.
why do we not simply throw an error if the eigenbasis is not predefined? No backend would know how to simulate it anyway.
There was a problem hiding this comment.
I'm not sure I get what you mean. Do you mean in the case that the eigenstates don't match one of these sets we would just throw an error right away?
There was a problem hiding this comment.
That sounds like something each subclass may chose to do, I don't want to enforce the supported eigenbasis in the ABC
There was a problem hiding this comment.
I'm not sure. For example, in this implementation, you're considering that the bases in lines 153-160 are known. We only support rg and 01 currently, and that is fine. But what is the use-case for supporting uv and having the user supply that u measures as 1? Perhaps in the qutip backend the state object is independent enough from the simulation that this works, even though no dynamics are really defined for it, but actually enforcing all backends to have a state so independent from the simulation is what seems excessive to me.
The main place where this manifests in an uncomfortable way for me is in the presence of the one_state arg in sample. We only support the rg and 01 bases currently, and these have well defined behaviour upon measurement that I don't think the user should override.
There was a problem hiding this comment.
The reason why I'm allowing this flexibility is because, already, we have a case where we can't infer the one state - notably, when we have the qutrit with eigenstates rgh, the one_state actually depends on the measurement basis.
I defined infer_one_state() to avoid having the user defining the one state as much as possible but sometimes it is unavoidable.
I think that if you want to block the definition of unknown eigenstates or custom one states, you can do so in MPS. As for the ABC, I think we should allow this flexibility. If we don't and at some point want to support some other basis in a particular backend, we would have to open a PR on pulser-core to change the base class, which doesn't make sense to me.
Put another way, the State ABC should not include a hardcoded supserset of the basis states supported by all backends.
HGSilveri
force-pushed
the
hs/backend-api
branch
from
f1aeb69 to
dc26f3b
Compare
| *, | ||
| eigenstates: Sequence[Eigenstate], | ||
| n_qudits: int, | ||
| operations: FullOp, |
There was a problem hiding this comment.
in the notion page we had an extra argument here operators: dict[str, QubitOp] where you could define aliases for certain QuditOps such as {"X": {"rg": 1, "gr": 1}}. I found this super useful and started depending on it in several places. Now I realize it's not in the method. Could we add it back in?
There was a problem hiding this comment.
Can't it be specfic to MPO? I'm not a big fan of it and chose not to have it in QutipOperator as I found it overcomplicated the implementation unnecessarily.
I think that what you wanted could be achieved by defining an auxiliary variable instead, eg.
X = {"rg": 1, "gr": 1}
QutipOperator.from_operator_repr(
eigenstates=("r", "g"),
n_qudits=3,
operations=[(1.0, [(X, {0, 2})]],
)
Resultsclass and makeResulta subclass of it in backwards compatible wayStateandOperatorABCsBackendConfigand definesEmulationConfigEmulatorBackendABCCallbackandObservableABCsTo do:
QutipStateto have an exampleQutipOperator