Defining the new Backend API classes #764
Conversation
| """ | ||
| return list(self._results[self._find_uuid(observable)].keys()) | ||
|
|
||
| def get_result(self, observable: Observable | str, time: float) -> Any: |
There was a problem hiding this comment.
There is an interesting feature request in emu-mps that is related to this.
Also this morning we discussed with Kemal that the "dict" data structure indexed by time is maybe not the most appropriate:
- most people use the data to make plots, where they need a list of values in chronological order
- few people request value at a given time individually, except the latest time (with a list
[-1]) - dict has probably a lot of overhead compared to list
There was a problem hiding this comment.
Should we specify that the _store method is called in increasing order of time for a given callback? In which case the dict entries are always sorted by time according to this
Keys and values are iterated over in insertion order.
There was a problem hiding this comment.
I agree with this, although we should still store the evaluation times of each observable in a separate list
There was a problem hiding this comment.
That sounds reasonable I think. Maybe allow sth like plt.plot(*results["energy"]), that is, return both times and values in unzipped format in the __getitem__?
There is also this thing that evaluation_times could just be a sort of observable dt/integer multiple of dt (or -1 for just the final time for instance), except if there is some strong use case for evaluating observables at non-regular times. In that case, it is not needed to store the times. At least all notebooks I saw of people using emu_mps API use a range of x multiples.
Sounds insignificant but when we get to serialize it for repeated usage in cloud, can save some traffic.
On the side, I think having a plot method for adequate observables would greatly help people working with simulation data. Throwing here some ideas:
results = MPSBackend().run(...)
plt.plot(*results["energy"])
# or
results["energy"].plot()
# or
results.plot("energy")
This plot method would be observable-specific and NotImplementedError by default in the base class
There was a problem hiding this comment.
That sounds reasonable I think. Maybe allow sth like
plt.plot(*results["energy"]), that is, return both times and values in unzipped format in the__getitem__?
I'm not sure about this, not all observables are plottable and I think it would make for a counter-intuitive return type for __getitem__().
(Side note: you might have noticed that I went for __getattr__ instead of __getitem__ - so it would be results.energy rather than results["energy"]. This is to avoid confusion between Results and Sequence[Results], which some backends return. )
There is also this thing that
evaluation_timescould just be a sort of observable dt/integer multiple of dt (or -1 for just the final time for instance), except if there is some strong use case for evaluating observables at non-regular times. In that case, it is not needed to store the times. At least all notebooks I saw of people using emu_mps API use a range of x multiples.
Sounds insignificant but when we get to serialize it for repeated usage in cloud, can save some traffic.
I don't feel comfortable enforcing this at the base class level. I'd rather take the (minor) hit of making the serialized object a bit larger, we won't be saving much there anyway.
On the side, I think having a
plotmethod for adequate observables would greatly help people working with simulation data. Throwing here some ideas:results = MPSBackend().run(...) plt.plot(*results["energy"]) # or results["energy"].plot() # or results.plot("energy")This plot method would be observable-specific and NotImplementedError by default in the base class
I'm not opposed to this but I think this is the sort of thing that would make more sense to implement first in a MPS specific Results subclass, for two main reasons:
- All users are currently interacting with
Resultsvia EMU-MPS - I suspect there will be some trial and error before the best format is found, so it's better for you to have the freedom and flexibility to make adjustments without relying on
pulser-core
Once the format is well-established, we can potentially move it up toResultsso it's shared by all backends.
|
@a-corni This is ready for review |
Resultsclass and makeResulta subclass of it in backwards compatible wayStateandOperatorABCsBackendConfigand definesEmulatorConfigEmulatorBackendABCCallbackandObservableABCsTo do:
QutipStateto have an exampleQutipOperator