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

Separate draw_samples and draw_sequence #533

Merged
merged 32 commits into from
Jun 19, 2023
Merged
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0f32556
separate draw_samples and draw_sequence
dakk Jun 7, 2023
daafc55
rebase
dakk Jun 7, 2023
4501e77
restore target rendering
dakk Jun 7, 2023
91166cf
fix linters and types
dakk Jun 7, 2023
2fb542d
move measurement drawing and draw_interp_pts to draw_sequenece
dakk Jun 8, 2023
76276e1
- add time_slots of type target to ChannelSamples
dakk Jun 9, 2023
9ee018f
Fix samples.py typing
dakk Jun 9, 2023
63ab9e4
remove useless comments
dakk Jun 9, 2023
807c8e8
- fix _seq_drawer parameters names
dakk Jun 9, 2023
0bcbf57
fix linting
dakk Jun 9, 2023
ab4f2e3
move optional register drawing to draw_samples
dakk Jun 9, 2023
09c580e
splitting of drawing of target regions
dakk Jun 12, 2023
1363f2c
fix linters
dakk Jun 12, 2023
b7b725f
move draw_phase_shifts outside the loop
dakk Jun 12, 2023
3f4afd4
remove duplicate code from draw_sequence
dakk Jun 12, 2023
6757303
separate draw_channel_content from draw_samples
dakk Jun 12, 2023
2bd97b6
restore _seq_drawer boxes definition position
dakk Jun 12, 2023
70b3d2c
minor edits on seq_drawer
dakk Jun 12, 2023
652484d
adapt draw_phase_area for using sampled_seq in _seq_drawer
dakk Jun 13, 2023
e89d060
- move gather_data to draw_channel_content
dakk Jun 13, 2023
0309a42
- add _basis_ref to SequenceSamples
dakk Jun 13, 2023
3f56a89
- move phase_str into _draw_channel_content
dakk Jun 13, 2023
69e9a1f
refactoring of _seq_drawer
dakk Jun 13, 2023
2a7cf7e
Refactoring of _seq_drawer.py
dakk Jun 13, 2023
ae3b974
fix typo
dakk Jun 13, 2023
4706247
fix EOM drawing in draw_sequence
dakk Jun 13, 2023
33b598e
remove useless if
dakk Jun 13, 2023
38834ab
- test_draw_samples
dakk Jun 16, 2023
f67d642
- add eom_start_buffers and eom_end_buffers in ChannelSamples
dakk Jun 19, 2023
a0c3692
preserve backward compatibility for _TargetSlot
dakk Jun 19, 2023
6c22203
Pin numpy version to < 1.25
dakk Jun 19, 2023
b39a25d
use eom_blocks for eom_intervals_ti creation
dakk Jun 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions pulser-core/pulser/sequence/_seq_drawer.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ def _give_curves_from_samples(
]


def gather_data(sampled_seq: SequenceSamples) -> dict:
def gather_data(
sampled_seq: SequenceSamples, shown_duration: Optional[int] = None
) -> dict:
"""Collects the whole sequence data for plotting.

Args:
Expand All @@ -172,8 +174,15 @@ def gather_data(sampled_seq: SequenceSamples) -> dict:
Returns:
The data to plot.
"""
n_channels = len(sampled_seq.channels)
if not n_channels:
raise RuntimeError("Can't draw an empty sequence.")

dakk marked this conversation as resolved.
Show resolved Hide resolved
# The minimum time axis length is 100 ns
total_duration = max(sampled_seq.max_duration, 100)
if shown_duration is not None:
total_duration = shown_duration
else:
total_duration = max(sampled_seq.max_duration, 100)
dakk marked this conversation as resolved.
Show resolved Hide resolved
data: dict[str, Any] = {}
for ch, ch_samples in sampled_seq.channel_samples.items():
target: dict[Union[str, tuple[int, int]], Any] = {}
Expand Down Expand Up @@ -239,11 +248,13 @@ def gather_data(sampled_seq: SequenceSamples) -> dict:
eom_end_buffers,
)

if sampled_seq._measurement is not None:
data["measurement"] = sampled_seq._measurement
data["total_duration"] = total_duration
return data


def draw_channel_content(
def _draw_channel_content(
data: dict,
dakk marked this conversation as resolved.
Show resolved Hide resolved
sampled_seq: SequenceSamples,
register: Optional[BaseRegister] = None,
Expand Down Expand Up @@ -563,12 +574,8 @@ def draw_samples(
draw_phase_curve: Draws the changes in phase in its own curve (ignored
if the phase doesn't change throughout the channel).
"""
n_channels = len(sampled_seq.channels)
if not n_channels:
raise RuntimeError("Can't draw an empty sequence.")

data = gather_data(sampled_seq)
(fig_reg, fig, ch_axes, data) = draw_channel_content(
(fig_reg, fig, ch_axes, data) = _draw_channel_content(
data,
sampled_seq,
register,
Expand Down Expand Up @@ -629,13 +636,10 @@ def phase_str(phi: float) -> str:
else:
return rf"{value:.2g}$\pi$"

n_channels = len(seq.declared_channels)
if not n_channels:
raise RuntimeError("Can't draw an empty sequence.")

# Sample the sequence and get the data to plot
sampled_seq = sample(seq)
data = gather_data(sampled_seq)
sampled_seq = sample(seq, modulation=draw_modulation)
dakk marked this conversation as resolved.
Show resolved Hide resolved
shown_duration = seq.get_duration(include_fall_time=draw_modulation)
dakk marked this conversation as resolved.
Show resolved Hide resolved
data = gather_data(sampled_seq, shown_duration)

# Gather additional data for sequence specific drawing
for ch, sch in seq._schedule.items():
Expand All @@ -656,14 +660,11 @@ def phase_str(phi: float) -> str:
if interp_pts:
data[ch].interp_pts = dict(interp_pts)

if hasattr(seq, "_measurement"):
data["measurement"] = seq._measurement

# Boxes for qubit and phase text
area_ph_box = dict(boxstyle="round", facecolor="ghostwhite", alpha=0.7)
ph_box = dict(boxstyle="round", facecolor="ghostwhite")

(fig_reg, fig, ch_axes, data) = draw_channel_content(
(fig_reg, fig, ch_axes, data) = _draw_channel_content(
data,
sampled_seq,
seq.register if draw_register else None,
Expand Down Expand Up @@ -732,7 +733,15 @@ def phase_str(phi: float) -> str:
np.sum(yseff[0][seq_.ti : seq_.tf]) * 1e-3 / np.pi
)
else:
area_val = seq_.type.amplitude.integral / np.pi
area_val = (
np.sum(
sampled_seq.channel_samples[ch].amp[
slot.ti : slot.tf
]
)
* 1e-3
/ np.pi
)
phase_val = seq_.type.phase
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[draw_phase_area] Phase is constant if not modulated, I propose you take phase_val as the phase at tf-1 (otherwise I guess it means that we definitely have to provide all the slots of ChannelSchedule to ChannelSamples as TimeSlots)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figure_1

does this seems a reasonable result? I used sampled_seq.channel_samples[ch].phase[slot.ti:slot.tf][-1]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the perspective of the phase area, it is great !
It is also working well from the phase value perspective, but I am noticing a difference between yours and the current version, your version is displaying all the phase whereas we are displaying only the phase shifts.
Nit: You can try sampled_seq.channel_samples[ch].phase[slot.tf-1], it should work as well (and be simpler)
image

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using phase_val = sampled_seq.channel_samples[ch].phase[slot.tf-1] I was getting outofbound; I fixed by passing shown_duration as extended_duration to sample().

The other issue (displaying all the phase) was a wrong condition, I think I've solved it in the next commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's weird, to me sampled_seq.channel_samples[ch].phase[slot.ti:slot.tf][-1] should be equal to sampled_seq.channel_samples[ch].phase[slot.tf-1] - and it's weird that one raises an error without the other 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not equal in python; [A:B] get a slice from index A to index B OR len() if B > len() (try [1,2][0:1000])

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks for reminding me ;)

x_plot = (seq_.ti + seq_.tf) / 2 / time_scale
if (
Expand Down
Loading