Conversation
There was a problem hiding this comment.
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.
| f = self._prep_qstackframe(self._bound_kvs(*args, **kwargs)) | ||
| return bb.add( |
There was a problem hiding this comment.
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.
| try: | ||
| outs = self.add_t(bloq, **in_soqs) | ||
| except BloqError as be: | ||
| # Error source shown as `bb.add(...)` | ||
| raise BloqError(*be.args) from None |
There was a problem hiding this comment.
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.
"tracing style" bloq construction. This is the basics. More syntactic sugar, docs, and examples to follow.