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

Interpolated waveforms can fail on negative amplitudes #590

Closed
CdeTerra opened this issue Oct 4, 2023 · 4 comments · Fixed by #592
Closed

Interpolated waveforms can fail on negative amplitudes #590

CdeTerra opened this issue Oct 4, 2023 · 4 comments · Fixed by #592
Labels
bug Something isn't working

Comments

@CdeTerra
Copy link
Collaborator

CdeTerra commented Oct 4, 2023

The following code will raise an exception:

params={'evolution_time': 1000, 'delta_list': [1, 5, 10]}

failing_seq = Sequence.from_abstract_repr('{"version": "1", "name": "pulser-exported", "register": [{"name": "0", "x": -0.7591895994030079, "y": 0.8206167789812338}, {"name": "1", "x": -0.5019358894634652, "y": 0.10113489652630593}, {"name": "2", "x": 0.26112548886647297, "y": -0.3107860414514182}, {"name": "3", "x": 1.0, "y": -0.6109656340561218}], "channels": {"ising": "rydberg_global"}, "variables": {"evolution_time": {"type": "float", "value": [0.0]}, "delta_list": {"type": "float", "value": [0.0, 0.0, 0.0]}}, "operations": [{"op": "pulse", "channel": "ising", "protocol": "min-delay", "post_phase_shift": 0, "amplitude": {"kind": "interpolated", "times": [0.0, 0.5, 1.0], "duration": {"expression": "index", "lhs": {"variable": "evolution_time"}, "rhs": 0}, "values": [1e-09, 25497040.632286467, 1e-09]}, "detuning": {"kind": "interpolated", "times": [0.0, 0.5, 1.0], "duration": {"expression": "index", "lhs": {"variable": "evolution_time"}, "rhs": 0}, "values": {"variable": "delta_list"}}, "phase": 0}], "measurement": null, "device": {"version": "1", "channels": [{"id": "rydberg_global", "basis": "ground-rydberg", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "rydberg_local", "basis": "ground-rydberg", "addressing": "Local", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": 0, "fixed_retarget_t": 0, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "raman_global", "basis": "digital", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "raman_local", "basis": "digital", "addressing": "Local", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": 0, "fixed_retarget_t": 0, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "mw_global", "basis": "XY", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}], "name": "DefaultDevice", "dimensions": 3, "rydberg_level": 70, "min_atom_distance": 0.001, "max_atom_num": null, "max_radial_distance": null, "interaction_coeff_xy": 3700.0, "supports_slm_mask": true, "max_layout_filling": 0.5, "reusable_channels": true, "is_virtual": true}}')

failing_seq.build(**params)

It raises ValueError: All samples of an amplitude waveform must be greater than or equal to zero.. It does so under Pulse.__init__, where it happens that the first and the last values of amplitude.samples are close to zero but negative: amplitude.samples[0]=1e-09 and amplitude.samples[-1]=-2e-09.

Changing a single value in the interpolation amplitude fixes it:

working_seq = Sequence.from_abstract_repr('{"version": "1", "name": "pulser-exported", "register": [{"name": "0", "x": -0.7591895994030079, "y": 0.8206167789812338}, {"name": "1", "x": -0.5019358894634652, "y": 0.10113489652630593}, {"name": "2", "x": 0.26112548886647297, "y": -0.3107860414514182}, {"name": "3", "x": 1.0, "y": -0.6109656340561218}], "channels": {"ising": "rydberg_global"}, "variables": {"evolution_time": {"type": "float", "value": [0.0]}, "delta_list": {"type": "float", "value": [0.0, 0.0, 0.0]}}, "operations": [{"op": "pulse", "channel": "ising", "protocol": "min-delay", "post_phase_shift": 0, "amplitude": {"kind": "interpolated", "times": [0.0, 0.5, 1.0], "duration": {"expression": "index", "lhs": {"variable": "evolution_time"}, "rhs": 0}, "values": [1e-09, 14288808.541256472, 1e-09]}, "detuning": {"kind": "interpolated", "times": [0.0, 0.5, 1.0], "duration": {"expression": "index", "lhs": {"variable": "evolution_time"}, "rhs": 0}, "values": {"variable": "delta_list"}}, "phase": 0}], "measurement": null, "device": {"version": "1", "channels": [{"id": "rydberg_global", "basis": "ground-rydberg", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "rydberg_local", "basis": "ground-rydberg", "addressing": "Local", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": 0, "fixed_retarget_t": 0, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "raman_global", "basis": "digital", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "raman_local", "basis": "digital", "addressing": "Local", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": 0, "fixed_retarget_t": 0, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}, {"id": "mw_global", "basis": "XY", "addressing": "Global", "max_abs_detuning": null, "max_amp": null, "min_retarget_interval": null, "fixed_retarget_t": null, "max_targets": null, "clock_period": 1, "min_duration": 1, "max_duration": null, "mod_bandwidth": null, "eom_config": null}], "name": "DefaultDevice", "dimensions": 3, "rydberg_level": 70, "min_atom_distance": 0.001, "max_atom_num": null, "max_radial_distance": null, "interaction_coeff_xy": 3700.0, "supports_slm_mask": true, "max_layout_filling": 0.5, "reusable_channels": true, "is_virtual": true}}')

working_seq.build(**params)

This time, the first and last values of amplitude.samples are close to zero and positive: amplitude.samples[0]=1e-09 and amplitude.samples[-1]=3e-09.

Isn't this "random" behavior a bug? Shouldn't the interpolation always take care of having positive values? A simple fix could be to round negative small values to zero.

@CdeTerra CdeTerra added the bug Something isn't working label Oct 4, 2023
@HGSilveri
Copy link
Collaborator

Isn't this "random" behavior a bug? Shouldn't the interpolation always take care of having positive values? A simple fix could be to round negative small values to zero.

There's probably something we can do about this, but it's not true that the interpolation should always take care of having positive values. A waveform can be used for detuning too, where negative values are perfectly valid.

Still, I can see from your sequence that your amplitude endpoints are not being respected (you are specifying [1e-09, 25497040.632286467, 1e-09]) so the waveform should not be negative on the edges. This looks like a bug to me, indeed.

@CdeTerra
Copy link
Collaborator Author

CdeTerra commented Oct 4, 2023

There's probably something we can do about this, but it's not true that the interpolation should always take care of having positive values. A waveform can be used for detuning too, where negative values are perfectly valid.

Yes, of course, I didn't mean to remove negative values altogether in all situations, but I intended in this specific situation where it shouldn't happen, hence the simple fix that I'm proposing. In this context, small negative values may simply be a matter of calculation rounding errors.

@HGSilveri
Copy link
Collaborator

These are indeed rounding errors stemming from the fact that your amplitude range is very high (unrealistically high, in fact). The values of the samples are in fact already being rounded, but the precision of the interpolator is always relative to the largest value handled. The value you are inputing here is so high that you are going over the limit.

The problem with rounding off "small" negative values is defining what is "small". Currently, we round to the 9th decimal place, so anything >=-0.5e9 is rounded to 0 (this is the current definition of small). We could reduce the precision to the 8th decimal place, but that won't prevent someone from trying an even higher amplitude range and running into the same issue.

@HGSilveri
Copy link
Collaborator

HGSilveri commented Oct 4, 2023

The only robust solution I can think of is to determine the number of decimals based on the size of the range of values. Do you see any drawbacks to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants