Update primitive options type #52
Conversation
There was a problem hiding this comment.
Thanks @jyu00 ! dataclasses are great. I'm not sold that the following is the best idea, but since I already did it, I figured I might as well post it here. The idea is to put a thin layer of magic (blegh) on top of dataclasses so that they are typed in a way we might like, and to support serialization/deserialization of nested options more naturally:
from dataclasses import asdict, dataclass, field, replace
from typing import Optional, get_type_hints
class OptionsType(type):
"""Metaclass for options classes.
Ensures all options have defaults. This is done to prevent non-locality of default values.
"""
def __new__(cls, name, bases, attrs):
# ensure an annotations dict exists
annotations = attrs["__annotations__"] = attrs.get("__annotations__", {})
# demand default values for all attributes
for attr in annotations:
if attr not in attrs:
raise ValueError(f"'{name}.{attr}' must be assigned a default value")
# allow sub-options to forgo specifying an annotation or using field() attributions
for attr in list(attrs):
val = attrs.get(attr)
if isinstance(val, OptionsType):
attrs[attr] = field(default_factory=val)
annotations[attr] = val
elif isinstance(type(val), OptionsType):
# just val.copy would copy any changes made to val between now and construction
attrs[attr] = field(default_factory=val.copy().copy)
annotations[attr] = type(val)
# construct the dataclass and return
return dataclass(super().__new__(cls, name, bases, attrs))
class Options(metaclass=OptionsType):
def copy(self):
return replace(self)
def to_dict(self):
# note: we could do something more complicated that skips default values, but then we run
# the risk of getting out of sync with another version's defaults if this dict is serde'd.
return asdict(self)
@classmethod
def from_dict(cls, options_dict):
if options_dict is None:
return cls()
hints = get_type_hints(cls)
return cls(**{
attr: hints[attr].from_dict(val) if isinstance(hints.get(attr), OptionsType) else val
for attr, val in options_dict.items()
})With example:
class TwirlingOptions(Options):
gates: bool = False
measure: bool = False
class TranspilationOptions(Options):
optimization_level: int = 3
coupling_map: None = None
class ExecutionOptions(Options):
max_circuits: Optional[int] = 900
shots: Optional[int] = 4096
samples: Optional[int] = None
shots_per_sample: Optional[int] = None
interleave_samples: bool = False
class EstimatorOptions(Options):
twirling = TwirlingOptions
transpilation = TranspilationOptions
execution = ExecutionOptions(shots=3)
There was a problem hiding this comment.
Can you speak to the motivation of why "options" should be included in the required interface of a primitive, and not entirely left to implementations? There's no individually defined options, so what is actually being gained; a user must consult the documentation of their implementation already, to know what each supported value means, because it's not defined here (and imo that's a good thing). The more that's defined, the less free implementations are to make things that are better; this is the current problem you're describing with the Qiskit Options class, which the current primitives require exists.
An interface should typically require only the absolute minimum set of methods/attributes that all implementers must have, and their semantics. Here, defining the Python type of options without specifying the options doesn't actually achieve very much; the semantics are still undefined. This RFC attempts to enforce various things such as "'options' should autocomplete", "'options' should behave under dir(options)", "'options' should behave under copy.{copy,deepcopy}", but those are the standard behaviours of well written Python objects (which Options isn't, really). It doesn't (and cannot) actually make the options transferable from one implementation of a primitive to another in a meaningful way, which is the purpose of an abstract interface.
If Aer and IBM Runtime / other IBM offerings want to define some shared options that they'll both accept and define the semantics of those (so there's a subset that you can transfer), that can be done separately to the base primitives interface in a different package / documentation entirely. You're also then much freer to upgrade that shared specification over time; you don't need to worry about the compatibility of other implementers, so we're not holding back IBM development.
|
Fwiw, I'm not at all opposed to defining a helper class somewhere that makes it easier to define nested dataclasses, I just don't think that the use of it should be enforced by the primitives interface. My (very much from the outside) view is that there's nothing Runtime or Aer are doing that isn't already served by standard usage of the implementations in one of |
This whole RFC can be summarized in one sentence - let's not use But if we don't use |
My whole spiel boils down to justifying "'drop |
|
+1 Pedantic https://docs.pydantic.dev. |
While I didn't feel too strongly about this, @ElePT brought up a good point that application developers who may encounter different implementation of the primitives would want a standard way to set/get options. |
|
I would support this view if there were any standardised options with defined semantics and types. As far as I'm aware, there's no plan for this, though, which means that the |
|
@jakelishman I assume you agree that most primitive implementations have need for some kind of optioning. I think we can also agree, as you argue, that they won't have standardized option names or values. Despite this, I think that there's still value to user experience in homogenizing how these options are exposed. It's very frustrating when one implementation's options have a copy() method, and another one's doesn't, and one has an update, and the other doesn't, and one has subscript lookup, and the other doesn't, and one has set_option() methods on the class, and the other doesn't etc, etc. Sure, one can go and look it up in the docs, but that doesn't address the usability. If you can just tab-complete to find the options you remember being there, it doesn't feel like a big deal (when you're hacking together a notebook, which is the most common user experience I expect) that the options for various implementations are different. |
I do, but that is true of almost all objects. The point of an interface is to abstract over differences between implementations for users. There's no concrete option being added here, so saying "the getters and setters are concrete" is no good, because they cannot be used in a concrete manner with only the abstract interface as the allowed names and the semantics of the options are not configurable. It constrains implementations into the future without enabling users, which means that when implementations want to do something that isn't representable well within
I agree that that's frustrating, but this RFC does not make it any simpler to update options between instances because it does not define the semantics of any options, nor what other state a primitive implementation may have. If anything, it makes it easier to silently do the wrong thing, by making it seem like the options can be accessed abstractly as part of the interface, when they cannot. Put this way: if you say that estimator1 = Estimator(...)
estimator2 = Estimator(dataclasses.asdict(estimator1.options))and have The point is that, as best as I can tell, specific implementations of If an interface is meant to enable particular user behaviours, it's not really enough to expect that all implementers will do what "feels natural" to us here; if we want a behaviour to be possible in the abstract (e.g. "a user should be able to construct a new version of a primitive in the abstract"), then the whole semantics of what that means need to be defined by the interface. Just doing bits of it (e.g. the
For the "algorithms designer" persona mentioned above as justification for the abstract options, I'd argue that this actually is the whole ballgame. It doesn't matter if you know the syntax for setting an option in a library if you don't know the semantics; it's still unsettable. The worst-case scenario is actually that it does set, then without warning does something different to what you expected. (Note I'm not at all suggesting making things deliberately different between implementations to avoid this, just in favour of not having an interface require that "similar" things look abstract when they're not.) When we talk about "[a user] hacking together a notebook" for tab completion, then we're not talking about a user of the abstract interface any more. I'd argue that tab completion is not something an abstract interface should be requiring. It's a suggestion for good usability, but requiring that everybody uses a base |
|
This is not a response, just a comment: all of @jakelishman 's arguments apply just as well to This is not an argument against unifying how various implementations choose to implement their (inevitable) options attributes, or, as @jakelishman said, "defining a helper class somewhere that makes it easier to define nested dataclasses". |
|
@jakelishman what if we want to make the minimal promise that users can define some options at instantiation of an |
|
If we define semantics for specific options and the effect that implementations must cause them to have, then I'm totally happy. If not, then we're still in a situation where a user can't pass I think the idea of "scoping" options between the object state and Just to be clear: I think the idea of getting rid of the current I should say too, that at this point in the process, if I couldn't talk you round to my view with my above comments, and I'm not reading anything so far that causes me to change my opinion right now, I don't necessarily think that ought to be a blocker on accepting the RFC as-is. I don't think it's the best possible interface, but I do think it's an acceptable interface, and I don't think there's any large technical risk from it - the worst case scenario to me is mostly that |
No description provided.