Skip to content

keep track of the subroutine definition order #52

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

Merged
merged 7 commits into from
Jul 11, 2023

Conversation

jcjaskula-aws
Copy link
Collaborator

The order of the subroutine definitions was based on their call order. We now update a list of function name at decorator call.
Only subroutines that have been called in the program will be added to prog.subroutines. The subroutines are then added to the program at the final serialization step in order given by prog._subroutine_definition_order.

Added a unit test.

@braised-babbage
Copy link
Contributor

Would it be possible to preserve the ordering of all declarations, regardless of type? For example, in OpenQASM a subroutine definition can refer to a global const declaration, but OpenQASM does require the const declaration to precede the subroutine definition. As a further extension to this mechanism, for OpenPulse we may wish to allow subroutines to refer to global definitions even if not const. It's also possible for a subroutine to have a gate application in its body, assuming the defcal precedes the subroutine definition. As it stands right now, the reordering (where subroutine definitions precede everything else) can make it hard to control the output.

@jcjaskula-aws
Copy link
Collaborator Author

jcjaskula-aws commented Jul 8, 2023

Would it be possible to preserve the ordering of all declarations, regardless of type?

Some variables are automatic declared at serialization for convenience. This might make strict ordering more difficult. Users could always force the declaration if they want to have it somewhere precisely.
That said subroutines are currently declared at the top by construction, but we might add a flag to the decorator to change this behavior.

@jcjaskula-aws jcjaskula-aws marked this pull request as ready for review July 10, 2023 18:22
@jcjaskula-aws jcjaskula-aws force-pushed the jcjaskula-aws/preserve_subroutine_order branch from 0a87f84 to 63a6ebd Compare July 10, 2023 19:48
Copy link
Collaborator

@PhilReinhold PhilReinhold 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 a few suggestions

@PhilReinhold
Copy link
Collaborator

Very nice, I like the new changes! It is much more consistent in how the declaration gets handled.

@jcjaskula-aws jcjaskula-aws merged commit e9ab80a into main Jul 11, 2023
@jcjaskula-aws jcjaskula-aws deleted the jcjaskula-aws/preserve_subroutine_order branch July 11, 2023 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants