Skip to content

Conversation

@charmasaur
Copy link
Contributor

@charmasaur charmasaur commented Sep 6, 2020

This covers the single qubit driven control library section of the wiki.

Changes are largely based on the wiki, although some of the references
have been fixed, and the description of WAMF1 now more accurately
matches the code.

The styling of the tables isn't great, but I'm hoping that's something that
can be fixed separately on the frontend side.

In a subsequent PR I'll make all these functions public, to be used instead
of new_predefined_driven_control.

Preview: https://qeng-1118.docs.q-ctrl.com/references/python/qctrl-open-controls/qctrlopencontrols.html
(for that preview I've made the functions public, but you get the idea)

https://q-ctrl.atlassian.net/browse/QENG-1118

@charmasaur charmasaur requested a review from a team September 6, 2020 06:33
Largely based on the wiki, although some of the references have been
fixed, and the description of WAMF1 now more accurately matches the
code.

https://q-ctrl.atlassian.net/browse/QENG-1118
Copy link
Member

@leoadec leoadec left a comment

Choose a reason for hiding this comment

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

Looks good, just one minor comment about style

Notes
-----
A BB1 driven control [#]_ consists of four control segments:
Copy link
Member

Choose a reason for hiding this comment

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

Oh wow, I didn't know [#]_ worked. Very neat!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's pretty cool. Could be an option for the api2 docs too (the converter could just convert all the [^n]'s into [#]_s).

"""
SCROFULOUS control to compensate for pulse length errors.
r"""
Creates a Short Composite ROtation For Undoing Length Over and Under Shoot (SCROFULOUS) driven
Copy link
Member

Choose a reason for hiding this comment

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

I think you don't need to "capitalize the spelled-out form of an acronym unless it's a proper noun" (https://docs.microsoft.com/en-us/style-guide/capitalization)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL, thanks!

def _new_primitive_control(
rabi_rotation: float,
azimuthal_angle: float = 0.0,
maximum_rabi_rate: float = 2.0 * np.pi,
Copy link
Member

Choose a reason for hiding this comment

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

doc translates np.pi to float number, can we disable that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem to be a clear way to do this, since Sphinx doesn't even see the fact that it's an np.pi (it gets expanded to its float value as soon as Python loads the module). See https://stackoverflow.com/questions/29257961/sphinx-documentation-how-to-disable-default-argument-expansion too.

tachikoma-li
tachikoma-li previously approved these changes Sep 7, 2020
Copy link
Member

@tachikoma-li tachikoma-li left a comment

Choose a reason for hiding this comment

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

looks good!

Copy link
Member

@leoadec leoadec left a comment

Choose a reason for hiding this comment

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

Looks good, just have one final tweak left, for consistency

Co-authored-by: Leonardo Andreta de Castro <leonardo.andreta@q-ctrl.com>
@charmasaur charmasaur merged commit 797fcf5 into master Sep 7, 2020
@charmasaur charmasaur deleted the pulses branch September 7, 2020 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants