Skip to content

Bloqify syntax basics#1839

Open
mpharrigan wants to merge 1 commit intoquantumlib:mainfrom
mpharrigan:2024-06/bloqify
Open

Bloqify syntax basics#1839
mpharrigan wants to merge 1 commit intoquantumlib:mainfrom
mpharrigan:2024-06/bloqify

Conversation

@mpharrigan
Copy link
Copy Markdown
Collaborator

"tracing style" bloq construction. This is the basics. More syntactic sugar, docs, and examples to follow.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the @bloqify decorator, which provides a function-based syntax for constructing CompositeBloq objects. To support this, BloqBuilder and CompositeBloq have been updated to include a bloq_key for better tracking and debugging. Additionally, the finalize method now supports a 'loose' mode that automatically converts missing THRU registers to LEFT registers. Review feedback highlights a potential performance issue where bloqified functions are re-traced on every call and suggests preserving exception context when re-raising BloqError to aid debugging.

Comment on lines +172 to +173
f = self._prep_qstackframe(self._bound_kvs(*args, **kwargs))
return bb.add(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current implementation of __call__ triggers a full trace of the bloqified function every time it is called as a sub-bloq. This can lead to significant performance overhead, especially when bloqified functions are used within loops or nested deep in a hierarchy. Consider caching the resulting CompositeBloq based on the classical arguments provided to the function to avoid redundant tracing.

Comment on lines +1470 to +1474
try:
outs = self.add_t(bloq, **in_soqs)
except BloqError as be:
# Error source shown as `bb.add(...)`
raise BloqError(*be.args) from None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Using raise BloqError(*be.args) from None suppresses the original exception context. While this might result in a cleaner error message for the user by pointing to the bb.add call, it makes debugging significantly harder as the internal traceback within add_t is lost. It is generally better to preserve the cause using from be unless there is a very strong reason to hide the implementation details.

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.

1 participant