Skip to content

Conversation

@wtbgagoa
Copy link
Member

This is a possible fix to #4 reported by Alejandro Jiménez Cano here.

@wtbgagoa wtbgagoa requested a review from duetosymmetry May 18, 2019 20:38
@wtbgagoa wtbgagoa self-assigned this May 18, 2019
@duetosymmetry
Copy link
Member

Thanks, Alfonso. I am wondering what is the right way to treat this bug. In a sense this is a deficiency of xTensor, not xTerior. Is it correct to create a fix here? What if xTensor gets enhanced, then will there be a conflict? Should we only apply such a fix if the xTensor version number is before a certain date?

@wtbgagoa
Copy link
Member Author

Indeed this is a xTensor issue so it is true that it should be fixed on the xTensor side, not here. The idea of this PR is that our users are able to work with symbolic grades until the issue gets solved in xTensor.
Perhaps we should add a check on the xTensor version as you suggest, and once the issue gets solved in xTensor then change the xTerior code accordingly. Another possibility is to decline this PR and just publish the workaround in the xAct group. Either possibility is fine for me.

@duetosymmetry
Copy link
Member

I like the idea of making this an "unofficial" fix on the mailing list, and addressing the xTensor problem. But I haven't thought about how xTensor's ToObject and checkgrade need to be generalized. Your suggestion

xAct`xTensor`Private`ToObject[xAct`xTensor`Private`AddedSign[(-1)^(deg_),expr_]]:=MapAt[xAct`xTensor`Private`VerbatimProduct[Times][(-1)^(deg),#1]&,xAct`xTensor`Private`ToObject[expr],1];

looks safe to me, but I don't know enough about the inner workings of the canonicalizer to be sure. So I defer to you and JMM on this topic. And what is the right way to approach checkgrade? Do we need to pass an optional CheckGrade function from xTerior to xTensor at the time we are defining the algebra? That is, do we need to generalize DefAlgebra to take an optional argument which it would use for checkgrade instead of the default one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants