Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposed fixes to discrepancy between linear transformation and matrix #469

Closed
Greg1950 opened this issue Oct 29, 2020 · 8 comments
Closed
Milestone

Comments

@Greg1950
Copy link
Contributor

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
Copy link
Member

utensil commented Oct 31, 2020

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
Copy link
Contributor Author

Greg1950 commented Nov 1, 2020

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 Greg1950 closed this as completed Nov 1, 2020
@Greg1950 Greg1950 reopened this Nov 2, 2020
@Greg1950
Copy link
Contributor Author

Greg1950 commented Nov 2, 2020

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
Copy link
Member

utensil commented Nov 3, 2020

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: #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
Symbolic Geometric Algebra/Calculus package for SymPy 🔮 - pygae/galgebra

@Greg1950
Copy link
Contributor Author

Greg1950 commented Nov 3, 2020 via email

@utensil
Copy link
Member

utensil commented Nov 9, 2020

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

@Greg1950
Copy link
Contributor Author

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 utensil added this to the 0.6.0 milestone Mar 30, 2024
@utensil
Copy link
Member

utensil commented May 15, 2024

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

@utensil utensil closed this as completed May 15, 2024
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

No branches or pull requests

2 participants