Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrize a sequence with items of lists #483

Open
CdeTerra opened this issue Mar 16, 2023 · 2 comments
Open

Parametrize a sequence with items of lists #483

CdeTerra opened this issue Mar 16, 2023 · 2 comments
Labels
archive Smthg to keep in mind, but not worth implementing now enhancement New feature or request

Comments

@CdeTerra
Copy link
Collaborator

On Pulser, it's possible to parametrize a sequence with a whole list. It's also possible to specify a single value with a list item. For example:

reg = Register(dict(enumerate([[0,0],[5,0]])))
seq = Sequence(reg, Chadoq2)
seq.declare_channel("ising", "rydberg_global")
time_list = seq.declare_variable("time_list", size=1, dtype=float)
amplitude_list = seq.declare_variable("amplitude_list", size=2, dtype=float)
pulse = Pulse(
    InterpolatedWaveform(
        time_list[0],              # list item
        amplitude_list),           # full list
    ConstantWaveform(time_list[0], 10),
    0
)
seq.add(pulse, "ising")
built_seq = seq.build(time_list=[20], amplitude_list=[0,10])

Providing a list constituted of items would also be nice:

pulse = Pulse(
    InterpolatedWaveform(
        time_list[0],
        [amplitude_list[0], 5, amplitude_list[1]]),    # list of items
    ConstantWaveform(time_list[0], 10),
    0
)
seq.add(pulse, "ising")
built_seq = seq.build(time_list=[20], amplitude_list=[0,10])

Unfortunately, it fails on the following error:

File ~/Documents/repos/Pulser/pulser-core/pulser/parametrized/paramobj.py:176, in <listcomp>(.0)
    173 self._vars_state = vars_state
    174 # Builds all Parametrized arguments before feeding them to cls
    175 args_ = [
--> 176     arg.build() if isinstance(arg, Parametrized) else arg
    177     for arg in self.args
    178 ]
    179 kwargs_ = {
    180     key: val.build() if isinstance(val, Parametrized) else val
    181     for key, val in self.kwargs.items()
    182 }
    183 if isinstance(self.cls, ParamObj):

File ~/Documents/repos/Pulser/pulser-core/pulser/parametrized/paramobj.py:187, in ParamObj.build(self)
    185     else:
    186         obj = self.cls
--> 187     self._instance = obj(*args_, **kwargs_)
    188 return self._instance

File ~/Documents/repos/Pulser/pulser-core/pulser/waveforms.py:762, in InterpolatedWaveform.__init__(self, duration, values, times, interpolator, **interpolator_kwargs)
    760 """Initializes a new InterpolatedWaveform."""
    761 super().__init__(duration)
--> 762 self._values = np.array(values, dtype=float)
    763 if times is not None:
    764     times = cast(ArrayLike, times)

TypeError: float() argument must be a string or a number, not 'VariableItem'

Could it be considered to implement this possibility? It would help a lot in cases like SciPy minimization when we want it to minimize specific items of a list, without having to construct and deconstruct the list.

Note: the following also fails for the same reason:

seq = Sequence(reg, Chadoq2)
seq.declare_channel("ising", "rydberg_global")
time_list = seq.declare_variable("time_list", size=1, dtype=float)
amplitude_start = seq.declare_variable("amplitude_start", dtype=float)
amplitude_end = seq.declare_variable("amplitude_end", dtype=float)
pulse = Pulse(
    InterpolatedWaveform(time_list[0], [amplitude_start, 5, amplitude_end]),
    ConstantWaveform(time_list[0], 10),
    0
)
seq.add(pulse, "ising")
built_seq = seq.build(time_list=[20], amplitude_start=0, amplitude_end=10)
@a-corni a-corni added the enhancement New feature or request label Mar 16, 2023
@HGSilveri
Copy link
Collaborator

HGSilveri commented Mar 22, 2023

Hi @CdeTerra ! I agree that this would be beneficial, but it's easier said than done... If my memory serves me right, I tried to do this at some point but ended giving up on the idea. I'll have to take a closer look to remind myself of the difficulties.

@HGSilveri
Copy link
Collaborator

Ok, I looked at it again. The main issue is that currently everything is setup assuming that the arguments to the Sequence calls are Parametrized instances.
This is why, for example, we turn Waveforms and Pulses into ParametrizedObj when they are instantiated with at least one Parametrized argument.

What you are suggesting is that we drop this assumption and start accepting that, although the argument of method is not parametrized (eg is a list), its elements might be. This will naturally increase the complexity of the creation and building process to a point that I'm not sure is absolutely necessary.

As supporting argument, the use case you presented above could be relatively easily circumvented through a wrapper around the sequence building call:

seq = Sequence(reg, Chadoq2)
seq.declare_channel("ising", "rydberg_global")
time_list = seq.declare_variable("time_list", size=1, dtype=float)
amplitudes= seq.declare_variable("amplitudes", dtype=float, size=3)
pulse = Pulse(
    InterpolatedWaveform(time_list[0], amplitudes),
    ConstantWaveform(time_list[0], 10),
    0
)
seq.add(pulse, "ising")

def get_built_seq(time_list, amplitude_start, amplitude_end):
    return seq.build(time_list=[20], amplitudes=[amplitude_start, 5, amplitude_end])

built_seq = get_built_seq(time_list=[20], amplitude_start=0, amplitude_end=10)

@HGSilveri HGSilveri added the archive Smthg to keep in mind, but not worth implementing now label Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive Smthg to keep in mind, but not worth implementing now enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants