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

Show all the specs of the Device #756

Open
a-corni opened this issue Oct 21, 2024 · 3 comments · May be fixed by #763
Open

Show all the specs of the Device #756

a-corni opened this issue Oct 21, 2024 · 3 comments · May be fixed by #763
Labels
good first issue Good for newcomers
Milestone

Comments

@a-corni
Copy link
Collaborator

a-corni commented Oct 21, 2024

Current _specs of the Device is only implemented for part of the attributes of the Device. For instance it does not display default_noise_model, requires_layout, accepts_new_layout, ...

@a-corni a-corni added the good first issue Good for newcomers label Oct 21, 2024
@HaroldErbin
Copy link
Contributor

I had a few questions related to this point:

  1. Why not store also the properties in a dict, in case we want to pass all properties (for example, to define a new device, save as json for easy check of the specs, etc.)? I find having a string not always convenient (also it's supposed to be an internal method).
  2. Why not implement _specs also for virtual devices? Following this tutorial, we could expect to be able to print the specs also for the derived virtual device.
  3. Why implement _specs as an internal method and not as a public property?

@HGSilveri
Copy link
Collaborator

I had a few questions related to this point:

1. Why not store also the properties in a `dict`, in case we want to pass all properties (for example, to define a new device, save as json for easy check of the specs, etc.)? I find having a string not always convenient (also it's supposed to be an internal method).

2. Why not implement `_specs` also for virtual devices? Following [this tutorial](https://pulser.readthedocs.io/en/stable/tutorials/virtual_devices.html), we could expect to be able to print the specs also for the derived virtual device.

3. Why implement `_specs` as an internal method and not as a public property?

The short answer is that the access and display of device specs has evolved somewhat carelessly. We need to improve the user interface, make sure it includes every parameter and that it is homogeneous between Device and VirtualDevice.
These are valuable suggestions, we'll keep them in mind!

@HaroldErbin
Copy link
Contributor

Note that I just found the _params() internal method, which does work for both physical and virtual devices. However, it does not contain all parameters (for example, interaction_coefficient is missing, while interaction_coefficient_xy is present). It could also better be a public property.

@HaroldErbin HaroldErbin linked a pull request Nov 19, 2024 that will close this issue
@HGSilveri HGSilveri linked a pull request Nov 21, 2024 that will close this issue
@HGSilveri HGSilveri added this to the v1.2 milestone Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants