search

kpeeters / cadabra2

A field-theory motivated approach to computer algebra.
https://cadabra.science/
GNU General Public License v3.0
228 stars 37 forks source link

Derivatives do not inherit SelfAntiCommuting properly in canonicalise #113

Open kpeeters opened 6 years ago

kpeeters commented 6 years ago

They do work in sort_product, so it is most likely a bug in core/Exchange.cc.

kpeeters commented 6 years ago

See https://cadabra.science/qa/908/derivatives-inheriting-selfanticommuting-canonicalise?show=909#a909 for a test case.

dpbutter commented 10 months ago

If I edit properties/Derivative.hh and add "public Inherit\<SelfCommutingBehaviour>" to the definition of "class Derivative", then canonicalise behaves as expected.

(In cadabra v1, the corresponding modification goes into modules/algebra.hh.)

kpeeters commented 10 months ago

Thanks for digging into this! I suspect that there are still issue lurking here (those comments about anti-commuting derivatives in the source point to problems which cannot really be solved with the current setup). But since your fix squashes the bug you reported and it does not break any of the other tests, I have pushed to github now.

dpbutter commented 10 months ago

Thanks! I also intended to mention that I noticed in pythoncdb/py_properties.cc that there is a line:

using Py_Derivative = BoundProperty<Derivative, Py_IndexInherit, Py_CommutingAsProduct, Py_NumericalFlat, Py_TableauBase, Py_Distributable, Py_WeightBase>;

I don't understand how the python bindings work, and I wasn't sure if this needed to be modified to track with the change in Derivative.cc.

kpeeters commented 10 months ago

To be consistent this should also include Py_SelfCommutingBehaviour. But those declarations are only there to make the property visible on the Python side; if it is not listed then you will simply not be able to check on the Python side whether a pattern with a a Derivative property has a SelfCommutingBehaviour attached to it.