pyzx icon indicating copy to clipboard operation
pyzx copied to clipboard

refactor: make scalar-handling backend agnostic

Open akissinger opened this issue 4 months ago • 2 comments

Currently the way PyZX handles scalars is not compatible with the Rust backend. The reason for this is Rust stores its scalars differently, so asking for the scalar from the python bindings returns a (lossy) copy of the current scalar. Hence, code like this which mutates the scalar doesn't work as expected:

g.scalar.add_power(5)  # no-op for the Rust backend

I suggest we deprecate mutating the scalar directly, and do this all via helper methods added to BaseGraph, which are added by this PR:

def mult_scalar_by_phase(self, phase: FractionLike) -> None:
    """Multiplies the scalar by a phase factor."""
    self.scalar.add_phase(phase)

def mult_scalar_by_sqrt2_power(self, power: int) -> None:
    """Multiplies the scalar by sqrt(2) raised to the given power."""
    self.scalar.add_power(power)

def mult_scalar_by_scalar(self, scalar: Scalar) -> None:
    """Multiplies scalar with the given scalar"""
    self.scalar.mult_with_scalar(scalar)

def mult_scalar_by_spider_pair(self, phase1: FractionLike, phase2: FractionLike) -> None:
    """Multiplies scalar with a 'spider pair', i.e. a pair of phased Z-spiders connected by an H edge"""
    self.scalar.add_spider_pair(phase1, phase2)

This lets the Rust backend override these methods to do the correct thing.

I will probably go ahead and merge this soon and also change all of the instances I find of mutating the scalar, since this shouldn't break anything. However, on the long term, we might want to look at e.g. renaming Basegraph.scalar to Basegraph._scalar to enforce using the helpers.

akissinger avatar Aug 27 '25 15:08 akissinger

@lara-madison and @jvdwetering can I go ahead and merge this? Hopefully it doesn't clobber anything else you are working on at the moment.

akissinger avatar Aug 27 '25 17:08 akissinger

That seems reasonable to me. I also agree that these names make more sense. We could also change the names of the corresponding method of the Scalar class, but that doesn't have to be in this PR. I think this doesn't break anything on our end so you could go ahead and merge.

jvdwetering avatar Aug 27 '25 18:08 jvdwetering