quantumlib / OpenFermion

The electronic structure package for quantum computers.
Apache License 2.0
1.51k stars 372 forks source link

Operation between MajoranaOperator and numbers? #862

Closed snow0369 closed 6 months ago

snow0369 commented 7 months ago

Although MajoranaOperator is not a subclass of SymbolicOperator, as explained in detail here, I believe that it should work with numbers. Currently, only the following methods accept a number as an operand: __mul__, __imul__, __rmul__, __truediv__, and __itruediv__.

I would like to understand if there is a specific rationale behind the absence of implementations for identity() and operations like __add__ with numbers.

Thank you.

fdmalone commented 7 months ago

What do you mean work with numbers exactly? @kevinsung implemented this so might have some more insights on the design decisions.

kevinsung commented 7 months ago

I would like to understand if there is a specific rationale behind the absence of implementations for identity() and operations like add with numbers.

As far as I remember, I don't think there was a specific rationale. Adding those methods simply wasn't deemed important enough at the time, but I think they would be a welcome addition.

snow0369 commented 7 months ago

What do you mean work with numbers exactly? @kevinsung implemented this so might have some more insights on the design decisions.

Sorry for the lack of the detail. I meant work with numbers as taking numbers (such as int, float and complex) as an input to the operations like __add__ and __sub__.

As far as I remember, I don't think there was a specific rationale. Adding those methods simply wasn't deemed important enough at the time, but I think they would be a welcome addition.

Then, I am happy to be assigned for that.

While I work on it, I think there is a bug in __isub__(here):

    def __isub__(self, other):
        if not isinstance(other, type(self)):
            return NotImplemented

        for term, coefficient in other.terms.items():
            if term in self.terms:
                self.terms[term] -= coefficient
            else:
                self.terms[term] = coefficient  # <- Here, shouldn't it be -coefficient?

        return self

Thank you.

snow0369 commented 6 months ago

@fdmalone Hi, I want to gently remind you about the pull request. I fixed a bug in the subtraction between two Majorana operators, and updated operation between Majorana operator and numbers. Thank you for your attention.

fdmalone commented 6 months ago

my bad. Thanks for the contribution.