Simplify the structure of Factory objects about mitiq HOT 7 CLOSED

4. I am slightly in favor of the current Factory mainly because it makes annotations more simple. However BaseFactory is also ok for me.
5. +1 to Factory.fetch (I also like Factory.pull and Factory.next ).

from mitiq.

My vote is for Factory as the base class, Factory.next, and Factory.push

I think Factory.pull usually indicates something that would change the internal state rather than just returning the next thing. Factory.push does actually change the internal state.

@rmlarose do you have other suggestions?

Great thoughts @andreamari ! IMO go for it on implementation

from mitiq.

Just Factory (and not BaseFactory) for the base class is good with me. (AbstractFactory kinda sounds like a cool progressive rock band name, though :)

However, thinking about it, the word factory is quite general -- when one first sees or hears the name, they probably won't have a good idea of what the object is. I'm definitely not saying we should rename it now with all the other naming considerations, just a thought.

I'm pretty neutral as to the other naming considerations.

For Andrea's point (1), I might still consider keeping instack and outstack as arguments to the constructor. My thinking is that: Suppose someone uses FactoryA to collect data, then wants to use a different extrapolation technique which is in FactoryB, or even collect more data with a different factory. They then would want to input FactoryA.instack and FactoryA.outstack to FactoryB.

from mitiq.

So with all these comments (module some method names), I think the signature for Factory should be something like:

class Factory:
  def __init__(self, scale_factors: Iterable[float], expectation_values: Iterable[float]) -> None:
    self._instack = scale_factors  # Or empty list if not provided
    self._outstack = expectation_values  # Or empty list if not provided

  def push(self, instack_value: float, outstack_value: float) -> None:
    """Pushes (appends) instack value to self._instack and outstack value to self._outstack."""

  def next(self) -> None:
    """Determines the next noise level to execute a circuit at (based on the expectation values computed thus far) and appends it to self._instack."""

  def is_converged(self) -> bool:
    """Returns True if all needed expectation values have been computed, else False."""

Thoughts? Did I get anything wrong?

By the way, editing this signature with different ideas might be easier to keep track of all comments/thoughts.

from mitiq.

• My feeling is that if one really needs to manually initialize instack and outstack,
  one can always access them as properties, e.g. factory.instack = x factory.outstack = y,
  while in a standard usage they should start as empty lists.
  However I am very flexible on this point.
• A more important point is that, in my opinion, it is better if instack and outstack are updated
  simultaneously by Factory.push() and not should not be changed by Factory.next() or
  other methods. The idea is that instack and outstack should represent only what has been
  actually measured up to now and not what we would like to measure in the future.

Based on the previous points, this would be my signature:

class Factory:
  def __init__(self, ...other arguments only on specific children classes) -> None:
      self.instack = [ ]
      self.outstack = [ ]

  def push(self, instack_value: float, outstack_value: float) -> None:
       """Pushes (appends) instack value to self.instack and outstack value to self.outstack."""

  def next(self) -> float:
       """Returns the next noise level to execute a circuit at.

  def is_converged(self) -> bool:
       """Returns True if all needed expectation values have been computed, else False."""

  def reduce(self) -> float:
       """Returns the zero-noise extrapolation"""

from mitiq.

Thanks for suggesting the signature @rmlarose. I agree with @andreamari 's comments above and the type signature.

from mitiq.

Looks good to me.

from mitiq.