proposed fixes to discrepancy between linear transformation and matrix

Greg1950 commented 4 years ago

Previously I have reported that the matrix of a linear transformation and the action of the transformation on basis vectors disagree. This happens both for specific and generic transformations, although the nature of the disagreement is not the same for the two types of transformation. I have made some slight modifications to lt.py which I believe eliminate the discrepancies. See the attached PDF of a Jupyter notebook.

Page 1: a blank page, not included
Page 2: a brief explanation of my terminology
Pages 3, 4: description of the problems and my proposed fixes
Pages 5--10: testing of my fixes. Discrepancies seem to be have been eliminated.

discrepancies between linear transformation and its matrix -- proposed fixes.pdf

utensil commented 4 years ago

Really appreciate it and it looks great to me! Do you plan to submit a PR based on it? Or you can also attach the python files and notebooks and I'll test it locally and try to make a PR.

Greg1950 commented 4 years ago

utensil, I'm sorry but I don't know what a "PR" is. I'm just an aging mathematics hobbyist who took a couple of edX introductory computer science courses a few years ago, and I'm not familiar with developers' jargon. But I think the contents of the attached zip file should serve.

The zip file contains my modification of GAlgebra module lt.py, the unofficial GAlgebra module gprinter.py (Alan Bromborsky, author), and two Jupyter notebooks. The notebooks, when run, test the modifications to lt.py. The file READ_THIS.pdf describes where in lt.py the modifications can be found.

proposed lt.py fixes.zip

Greg1950 commented 4 years ago

Closed the issue by accident. See the zip file attached to my post on 2020-10-31 for files which contain my proposed fixes and which test those fixes.

utensil commented 4 years ago

Oh, sorry for the abbreviation, PR stands for "pull request", which means that you can fork https://github.com/pygae/galgebra and create a few commits with your additional files and/or modifications and create a "ticket" showing the changed contents and the maintainers (@eric-wieser and me) can review, request changes and merge it into GAlgebra eventually. But I can do that with the attached files for you and we can continue the discussions there.

Here's an example of a (merged) PR: https://github.com/pygae/galgebra/pull/468 . You can see the author of the PR is @eric-wieser , who is also a maintainer, it means that he has the permission to directly commit into GAlgebra, but still he created a PR so that we could collaborate in the form of code review and discussion, this is the way of open source world.

GitHub
pygae/galgebra
Symbolic Geometric Algebra/Calculus package for SymPy :crystal_ball: - pygae/galgebra

Greg1950 commented 4 years ago

Utensil Song,

Perhaps in the future I will have sufficient time to learn the procedures you listed for a "pull request", but right now I am very busy, a state which will continue for at least the next week.

So I will take you up on your offer to do the necessary actions with the files I packaged together in proposed lt.py fixes.zip (attached to my post of 2020-10-31).

Thank you, Greg Grunberg

On Mon, Nov 2, 2020 at 6:13 PM Utensil Song notifications@github.com wrote:

Oh, sorry for the abbreviation, PR stands for "pull request", which means that you can fork https://github.com/pygae/galgebra and create a few commits with your additional files and/or modifications and create a "ticket" showing the changed contents and the maintainers (@eric-wieser https://github.com/eric-wieser and me) can review, request changes and merge it into GAlgebra eventually. But I can do that with the attached files for you and we can continue the discussions there.

Here's an example of a (merged) PR: #468 https://github.com/pygae/galgebra/pull/468 . You can see the author of the PR is @eric-wieser https://github.com/eric-wieser , who is also a maintainer, it means that he has the permission to directly commit into GAlgebra, but still he created a PR so that we could collaborate in the form of code review and discussion, this is the way of open source world.

https://avatars0.githubusercontent.com/u/33729520?s=400&v=4

https://camo.githubusercontent.com/040b5519c6d1891329f5f6d3e511003657131cf2/68747470733a2f2f6769746875622e6769746875626173736574732e636f6d2f66617669636f6e732f66617669636f6e2e737667 GitHub pygae/galgebra https://github.com/pygae/galgebra Symbolic Geometric Algebra/Calculus package for SymPy 🔮 - pygae/galgebra

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/pygae/galgebra/issues/469#issuecomment-720797703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG2OCIBSPFPGCP6PBLYNR2DSN5DKBANCNFSM4TDBQOLQ .

utensil commented 4 years ago

Yeah, we'll get to it ASAP (personally I just had a busy week and weekend, so I hadn't got to it).

Greg1950 commented 3 years ago

In my original posting I included an attachment, Discrepancies between a linear tranformation and its matrix -- proposed fixes.pdf.

I wish to take back my suggestion made therein for modifying the string input segment of the __init__ function for the Lt class. I would now leave unchanged the lines

        elif isinstance(mat_rep, str):  # String input
            Amat = Symbolic_Matrix(mat_rep, coords=self.Ga.coords, mode=mode, f=f)
            self.__init__(Amat, ga=self.Ga)

I had determined that the fragment's third line was the problem, but my solution was a kludge.

There is a much simpler fix. The fragment's third line invokes __init__ with a matrix argument, Amat. So look at the matrix input section of __init__:

        elif isinstance(mat_rep, Matrix):  # Matrix input
            self.mat = mat_rep
            mat_rep = self.mat * self.Ga.g_inv
            self.lt_dict = Matrix_to_dictionary(mat_rep, self.Ga.basis)

The problem was that the basis expansion of L(e_j) had coefficients which were not the matrix elements but rather the elements of the matrix right multiplied by the reciprocal metric tensor. We see in the second fragment's third line exactly that situation: The transformation's matrix, self.mat, is multiplied by the reciprocal metric tensor self.Ga.g_inv. And then in the fourth line that matrix product is passed on to the dictionary that maps e_j to L(e_j).

While I do not show it here, testing shows that deletion of the second and third lines of the second code fragment

        elif isinstance(mat_rep, Matrix):  # Matrix input
            self.lt_dict = Matrix_to_dictionary(mat_rep, self.Ga.basis)

corrects the problem of incorrect L(e_j) values for symbolic transformations L.

utensil commented 6 months ago

Thank you, @Greg1950 , I believe this is covered by your improvements merged by #510 . Feel free to reopen if there are remaining issues.

pygae / galgebra

proposed fixes to discrepancy between linear transformation and matrix #469