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

Add units to device parameter #6140

Merged

Conversation

dstrain115
Copy link
Collaborator

- Add the ability to add unit type for a device parameter.
- This will be important for server-side deserialization, where the server will need to know what units the float values are in.
- This will allow us to do sweeps of parameters that have units, such as nanoseconds, or GigaHertz

- Add the ability to add unit type for a device parameter.
- This will be important for server-side deserialization, where the server will need to know what units the float values are in.
- This will allow us to do sweeps of parameters that have units,
such as nanoseconds, or GigaHertz.
Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

LGTM

@CirqBot CirqBot added the size: S 10< lines changed <50 label Jun 13, 2023
@dstrain115 dstrain115 enabled auto-merge (squash) June 13, 2023 16:00
"""

path: Sequence[str]
idx: Optional[int] = None
value: Optional[Any] = None
units: Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a plan for exposing the spec for units? i.e. how do users know the valid options here? If there's a limited subset of Pyle units that we support, might it be worth providing those explicitly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To answer this comment a bit belatedly, there isn't an immediate plan to expose the units. However, there is also no plan to expose the valid path either. Users will generally need an expert internal user in order to use this feature anyway.

The valid spec is to use labrad units, which we may eventually expose, but this is a longer term project. We probably should revisit this when we start looking at how we want to integrate services like reg/DV with cirq_google.

@dstrain115 dstrain115 merged commit d236276 into quantumlib:master Jun 13, 2023
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants