Dagger() * IdentityOperator() now simplifies by default

[dhruvmendiratta6]

References to other Issues or PRs

Fixes #19747

Brief description of what is fixed or changed

from sympy.physics.quantum.dagger import Dagger
from sympy.physics.quantum.operator import Operator
from sympy.physics.quantum import IdentityOperator
A = Operator('A')
Identity = IdentityOperator()
print(B * Identity) 
print(Identity * B)

now gives

Dagger(A)
Dagger(A)

Other comments

Release Notes

• physics.quantum
  • Simplification of Dagger() * IdentityOperator()

[sympy-bot]

✅

Hi, I am the SymPy bot (v160). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

• physics.quantum
  • Simplification of Dagger() * IdentityOperator() (#19783 by @dhruvmendiratta6)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
Fixes #19747
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
```
from sympy.physics.quantum.dagger import Dagger
from sympy.physics.quantum.operator import Operator
from sympy.physics.quantum import IdentityOperator
A = Operator('A')
Identity = IdentityOperator()
print(B * Identity) 
print(Identity * B)
```
now gives
```
Dagger(A)
Dagger(A)
```



#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* physics.quantum
    * Simplification of Dagger() * IdentityOperator() 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[codecov]

Codecov Report

Merging #19783 into master will decrease coverage by 3.530%.
The diff coverage is 90.909%.

@@              Coverage Diff              @@
##            master    #19783       +/-   ##
=============================================
- Coverage   75.703%   72.173%   -3.531%     
=============================================
  Files          662       663        +1     
  Lines       172162    172247       +85     
  Branches     40604     40618       +14     
=============================================
- Hits        130332    124316     -6016     
- Misses       36138     42066     +5928     
- Partials      5692      5865      +173     

[namannimmo10]

Can you add more tests for this line?

[dhruvmendiratta6]

will this be fine

Dagger(A) * Identity * A == Dagger(A)*A
Identity * A * Dagger(A) == A*Dagger(A)

[namannimmo10]

You can apply this diff:

diff --git a/sympy/physics/quantum/tests/test_dagger.py b/sympy/physics/quantum/tests/test_dagger.py
index bcc8a28f58..54298c7ba2 100644
--- a/sympy/physics/quantum/tests/test_dagger.py
+++ b/sympy/physics/quantum/tests/test_dagger.py
@@ -1,6 +1,7 @@
-from sympy import I, Matrix, symbols, conjugate, Expr, Integer
+from sympy import I, Matrix, symbols, conjugate, Expr, Integer, Mul
 
 from sympy.physics.quantum.dagger import adjoint, Dagger
+from sympy.physics.quantum.operator import Operator, IdentityOperator
 from sympy.external import import_module
 from sympy.testing.pytest import skip
 
@@ -29,6 +30,15 @@ def test_matrix():
     assert Dagger(m) == m.H
 
 
+def test_dagger_mul():
+    O = Operator('O')
+    I = IdentityOperator()
+    assert Dagger(O)*O == Dagger(O)*O
+    assert Dagger(O)*O*I == Mul(Dagger(O), O)*I
+    assert Dagger(O)*Dagger(O) == Dagger(O)**2
+    assert Dagger(O)*Dagger(I) == Dagger(O)
+
+
 class Foo(Expr):
 
     def _eval_adjoint(self):

[namannimmo10]

Suggested change

[namannimmo10]

We should have a Release Notes entry for this PR.

[oscarbenjamin]

Is there a superclass of Operator and Dagger that would make more sense here? Should Dagger be a subclass of Operator?

I don't know the quantum code so well but special casing types like this suggests that things can be better organised somehow.

[namannimmo10]

I agree things can be better organized. Also: not every functionality is tested properly.

[dhruvmendiratta6]

Is there a superclass of Operator and Dagger that would make more sense here? Should Dagger be a subclass of Operator?

This is exactly what I was thinking and suggested on the mailing list. But while working on it, it turns out if such a change is made print doesn't work properly. print(Dagger(A)) gives A. I think this happens due to calling of _print_contents of Operator. And due to unfamiliarity with the printing module I found it difficult to further look into it

[oscarbenjamin]

I think that if Dagger is a subclass of Operator then if you add a _print_Dagger method it will take precedence over _print_Operator.

[dhruvmendiratta6]

Apologies for not being able to reply earlier, I tried overriding _print_contents() and made a separate _print() function for dagger but still it didn't work.

[namannimmo10]

I tried overriding _print_contents() and made a separate _print() function for dagger but still it didn't work

Can you show us the diff? It would be easier to comment after that. I think it should work if you add a new _print_Dagger() method.

[namannimmo10]

LGTM!

[oscarbenjamin]

Looks good. Merging