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

Update displayed device specs #763

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
135 changes: 83 additions & 52 deletions pulser-core/pulser/devices/_device_datacls.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from collections import Counter
from collections.abc import Mapping
from dataclasses import dataclass, field, fields
from typing import Any, Literal, cast, get_args
from typing import Any, Callable, Literal, cast, get_args

import numpy as np
from scipy.spatial.distance import squareform
Expand Down Expand Up @@ -591,71 +591,65 @@ def specs(self) -> str:
"""Text summarizing the specifications of the device."""
return self._specs(for_docs=False)

def _specs(self, for_docs: bool = False) -> str:
def yes_no_fn(cond: bool) -> str:
return "Yes" if cond else "No"
def _param_yes_no(self, param: Any) -> str:
return "Yes" if param is True else "No"

def _param_check_none(self, param: Any) -> Callable[[str], str]:
def empty_str_if_none(line: str) -> str:
if param is None:
return ""
else:
return line.format(param)

return empty_str_if_none

lines = [
def _register_lines(self) -> str:

register_lines = [
"\nRegister parameters:",
f" - Dimensions: {self.dimensions}D",
f" - Rydberg level: {self.rydberg_level}",
f" - Maximum number of atoms: {self.max_atom_num}",
f" - Maximum distance from origin: {self.max_radial_distance} μm",
(
" - Minimum distance between neighbouring atoms: "
f"{self.min_atom_distance} μm"
self._param_check_none(self.max_atom_num)(
" - Maximum number of atoms: {}"
),
f" - Maximum layout filling fraction: {self.max_layout_filling}",
f" - SLM Mask: {yes_no_fn(self.supports_slm_mask)}",
self._param_check_none(self.max_radial_distance)(
" - Maximum distance from origin: {} µm"
),
" - Minimum distance between neighbouring atoms: "
+ f"{self.min_atom_distance} μm",
f" - SLM Mask: {self._param_yes_no(self.supports_slm_mask)}",
]

if self.max_sequence_duration is not None:
lines.append(
" - Maximum sequence duration: "
f"{self.max_sequence_duration} ns"
)
return "\n".join(line for line in register_lines if line != "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we return list[str] instead? This (and the other similar methods) could change to

Suggested change
return "\n".join(line for line in register_lines if line != "")
return [line for line in register_lines if line != ""]


def _device_lines(self) -> str:

device_lines = [
"\nDevice parameters:",
]

if self.max_runs is not None:
device_lines.append(f" - Maximum number of runs: {self.max_runs}")
device_lines += [
f" - Channels can be reused: {yes_no_fn(self.reusable_channels)}",
self._param_check_none(self.max_runs)(
" - Maximum number of runs: {}"
),
self._param_check_none(self.max_sequence_duration)(
" - Maximum sequence duration: {} ns",
),
" - Channels can be reused: "
+ self._param_yes_no(self.reusable_channels),
f" - Supported bases: {', '.join(self.supported_bases)}",
f" - Supported states: {', '.join(self.supported_states)}",
self._param_check_none(self.interaction_coeff)(
" - Ising interaction coefficient: {}",
),
self._param_check_none(self.interaction_coeff_xy)(
" - XY interaction coefficient: {}",
),
self._param_check_none(self.default_noise_model)(
" - Default noise model: {}",
),
]

if self.interaction_coeff is not None:
device_lines.append(
f" - Ising interaction coefficient: {self.interaction_coeff}"
)

if self.interaction_coeff_xy is not None:
device_lines.append(
f" - XY interaction coefficient: {self.interaction_coeff_xy}"
)

if self.default_noise_model is not None:
device_lines.append(
f" - Default noise model: {self.default_noise_model}"
)

layout_lines = [
"\nLayout parameters:",
f" - Requires layout: {yes_no_fn(self.requires_layout)}",
]

if hasattr(self, "accepts_new_layouts"):
layout_lines.append(
f" - Accepts new layout: {yes_no_fn(self.accepts_new_layouts)}"
)
return "\n".join(line for line in device_lines if line != "")

layout_lines += [
f" - Minimal number of traps: {self.min_layout_traps}",
f" - Maximal number of traps: {self.max_layout_traps}",
]
def _channel_lines(self, for_docs: bool = False) -> str:

ch_lines = ["\nChannels:"]
for name, ch in {**self.channels, **self.dmm_channels}.items():
Expand Down Expand Up @@ -708,7 +702,17 @@ def yes_no_fn(cond: bool) -> str:
else:
ch_lines.append(f" - '{name}': {ch!r}")

return "\n".join(lines + device_lines + layout_lines + ch_lines)
return "\n".join(line for line in ch_lines if line != "")

def _specs(self, for_docs: bool = False) -> str:

return "\n".join(
[
self._register_lines(),
self._device_lines(),
self._channel_lines(for_docs=for_docs),
]
)
Comment on lines +709 to +715
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would then become

Suggested change
return "\n".join(
[
self._register_lines(),
self._device_lines(),
self._channel_lines(for_docs=for_docs),
]
)
return "\n".join(
self._register_lines()
+ self._device_lines()
+ self._channel_lines(for_docs=for_docs)
)



@dataclass(frozen=True, repr=False)
Expand Down Expand Up @@ -886,6 +890,33 @@ def from_abstract_repr(obj_str: str) -> Device:
)
return device

def _layout_lines(self) -> str:

layout_lines = [
"\nLayout parameters:",
f" - Requires layout: {self._param_yes_no(self.requires_layout)}",
" - Accepts new layout: "
+ self._param_yes_no(self.accepts_new_layouts),
f" - Minimal number of traps: {self.min_layout_traps}",
self._param_check_none(self.max_layout_traps)(
" - Maximal number of traps: {}"
),
f" - Maximum layout filling fraction: {self.max_layout_filling}",
]

return "\n".join(line for line in layout_lines if line != "")
Comment on lines +893 to +907
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had something slightly different in mind for the layout parameters. I think the ones that are in VirtualDevice should also appear in its specs, so this method could be moved to BaseDevice expect for the accepts_new_layout line. That one, we could add it like this:

Suggested change
def _layout_lines(self) -> str:
layout_lines = [
"\nLayout parameters:",
f" - Requires layout: {self._param_yes_no(self.requires_layout)}",
" - Accepts new layout: "
+ self._param_yes_no(self.accepts_new_layouts),
f" - Minimal number of traps: {self.min_layout_traps}",
self._param_check_none(self.max_layout_traps)(
" - Maximal number of traps: {}"
),
f" - Maximum layout filling fraction: {self.max_layout_filling}",
]
return "\n".join(line for line in layout_lines if line != "")
def _layout_lines(self) -> list[str]:
layout_lines = super()._layout_lines()
layout_lines.insert(2, f" - Accepts new layout: {self._param_yes_no(self.accepts_new_layouts)}"
return layout_lines


def _specs(self, for_docs: bool = False) -> str:

return "\n".join(
[
self._register_lines(),
self._layout_lines(),
self._device_lines(),
self._channel_lines(for_docs=for_docs),
]
)
Comment on lines +909 to +918
Copy link
Collaborator

@HGSilveri HGSilveri Nov 22, 2024

Choose a reason for hiding this comment

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

This (or rather its new version) would then be in BaseDevice and we don't need to redefine it here



@dataclass(frozen=True)
class VirtualDevice(BaseDevice):
Expand Down
59 changes: 38 additions & 21 deletions tests/test_devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ def test_tuple_conversion(test_params):
assert dev.channel_ids == ("custom_channel",)


def test_device_specs():
@pytest.mark.parametrize(
"device", [MockDevice, AnalogDevice, DigitalAnalogDevice]
)
def test_device_specs(device):
def yes_no_fn(dev, attr, text):
if hasattr(dev, attr):
cond = getattr(dev, attr)
Expand All @@ -275,46 +278,60 @@ def check_none_fn(dev, attr, text):
return ""

def specs(dev):
return (
register_str = (
"\nRegister parameters:\n"
+ f" - Dimensions: {dev.dimensions}D\n"
+ f" - Rydberg level: {dev.rydberg_level}\n"
+ f" - Maximum number of atoms: {dev.max_atom_num}\n"
+ " - Maximum distance from origin: "
+ f"{dev.max_radial_distance} μm\n"
+ check_none_fn(dev, "max_atom_num", "Maximum number of atoms: {}")
+ check_none_fn(
dev,
"max_radial_distance",
"Maximum distance from origin: {} µm",
)
+ " - Minimum distance between neighbouring atoms: "
+ f"{dev.min_atom_distance} μm\n"
+ f" - Maximum layout filling fraction: {dev.max_layout_filling}\n"
+ yes_no_fn(dev, "supports_slm_mask", "SLM Mask")
)

layout_str = (
"\nLayout parameters:\n"
+ yes_no_fn(dev, "requires_layout", "Requires layout")
+ yes_no_fn(dev, "accepts_new_layouts", "Accepts new layout")
+ f" - Minimal number of traps: {dev.min_layout_traps}\n"
+ check_none_fn(
dev, "max_layout_traps", "Maximal number of traps: {}"
)
+ f" - Maximum layout filling fraction: {dev.max_layout_filling}\n"
)

device_str = (
"\nDevice parameters:\n"
+ check_none_fn(dev, "max_runs", "Maximum number of runs: {}")
+ check_none_fn(
dev,
"max_sequence_duration",
"Maximum sequence duration: {} ns",
)
+ "\nDevice parameters:\n"
+ check_none_fn(dev, "max_runs", "Maximum number of runs: {}")
+ yes_no_fn(dev, "reusable_channels", "Channels can be reused")
+ f" - Supported bases: {', '.join(dev.supported_bases)}\n"
+ f" - Supported states: {', '.join(dev.supported_states)}\n"
+ f" - Ising interaction coefficient: {dev.interaction_coeff}\n"
+ check_none_fn(
dev, "interaction_coeff_xy", "XY interaction coefficient: {}"
)
+ "\nLayout parameters:\n"
+ yes_no_fn(dev, "requires_layout", "Requires layout")
+ yes_no_fn(dev, "accepts_new_layouts", "Accepts new layout")
+ f" - Minimal number of traps: {dev.min_layout_traps}\n"
+ f" - Maximal number of traps: {dev.max_layout_traps}\n"
+ "\nChannels:\n"
+ "\n".join(
f" - '{name}': {ch!r}"
for name, ch in {**dev.channels, **dev.dmm_channels}.items()
)
)

assert MockDevice.specs == specs(MockDevice)
assert AnalogDevice.specs == specs(AnalogDevice)
assert DigitalAnalogDevice.specs == specs(DigitalAnalogDevice)
channel_str = "\nChannels:\n" + "\n".join(
f" - '{name}': {ch!r}"
for name, ch in {**dev.channels, **dev.dmm_channels}.items()
)

if device is MockDevice:
return register_str + device_str + channel_str
else:
return register_str + layout_str + device_str + channel_str

assert device.specs == specs(device)


def test_valid_devices():
Expand Down