Bloq namespacing and display (__str__) updates.#1825
Bloq namespacing and display (__str__) updates.#1825mpharrigan wants to merge 4 commits intoquantumlib:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request is a nice improvement for the library, enhancing bloq namespacing and making __str__ representations more consistent and Python-like. The changes are well-structured and include relevant tests. I have two suggestions for improvement. First, to maintain consistency across the basic rotation gates, I recommend adding a CRx bloq and a specialized get_ctrl_system for Rx, similar to what has been done for Ry in this PR. Second, the new wire_symbol implementation in the Power bloq could be made more robust to handle different types of wire symbols for bloq titles. I've provided a code suggestion for a cleaner and more resilient implementation.
| def get_ctrl_system(self, ctrl_spec: 'CtrlSpec') -> Tuple['Bloq', 'AddControlledT']: | ||
| if ctrl_spec != CtrlSpec(): | ||
| return super().get_ctrl_system(ctrl_spec) | ||
|
|
||
| from qualtran.bloqs.mcmt.specialized_ctrl import get_ctrl_system_1bit_cv_from_bloqs | ||
|
|
||
| return get_ctrl_system_1bit_cv_from_bloqs( | ||
| bloq=self, | ||
| ctrl_spec=ctrl_spec, | ||
| current_ctrl_bit=None, | ||
| bloq_with_ctrl=CRy(self.angle, eps=self.eps), | ||
| ctrl_reg_name='ctrl', | ||
| ) |
There was a problem hiding this comment.
This is a great addition! For consistency with Ry and Rz, it would be good to also add a specialized get_ctrl_system for Rx. This would involve creating a CRx bloq, similar to how CRy was added in this PR. The _controlled_rp_circuit helper is already generic enough to support this.
You could add the following CRx class:
@frozen
class CRx(Bloq):
r"""A controlled Rx rotation."""
angle: SymbolicFloat
eps: SymbolicFloat = 1e-11
@cached_property
def signature(self) -> 'Signature':
return Signature.build(ctrl=1, q=1)
def build_composite_bloq(
self, bb: 'BloqBuilder', ctrl: 'Soquet', q: 'Soquet'
) -> Dict[str, 'SoquetT']:
return _controlled_rp_circuit(
bb, single_q_pow_cls=XPowGate, angle=self.angle, eps=self.eps, ctrl=ctrl, q=q
)
def __str__(self):
return f'CRx({self.angle})'And then add this method to the Rx class:
def get_ctrl_system(self, ctrl_spec: 'CtrlSpec') -> Tuple['Bloq', 'AddControlledT']:
if ctrl_spec != CtrlSpec():
return super().get_ctrl_system(ctrl_spec)
from qualtran.bloqs.mcmt.specialized_ctrl import get_ctrl_system_1bit_cv_from_bloqs
return get_ctrl_system_1bit_cv_from_bloqs(
bloq=self,
ctrl_spec=ctrl_spec,
current_ctrl_bit=None,
bloq_with_ctrl=CRx(self.angle, eps=self.eps),
ctrl_reg_name='ctrl',
)
As part of #1824 , I want to be better about our convention for importing bloqs and
__str__representations in the qualtran standard library (i.e.qualtran.bloqs). There's no additional restriction on custom, user-authored bloqs. These changes make sure:ControlledAddOrSubtractis defined inqualtran/bloqs/arithmetic/controlled_add_or_subtract.pybut it prefers to be imported fromqualtran.bloqs.arithmetic. This is codified in the (existing) default_pkg_(cls) -> strclassmethod. You can override it if you want something different.__str__representation looks like an object string (cc Qualtran-L1: Objectstrings #1823). This PR removes some special symbols in favor of more Python-looking strings.note that this causes more bloq examples to be loadable (visible in the re-generated manifest)