-
-
Notifications
You must be signed in to change notification settings - Fork 601
unitary DFT for symmetric group algebra #38455
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
unitary DFT for symmetric group algebra #38455
Conversation
Documentation preview for this PR (built with commit bf00417; changes) is ready! 🎉 |
a8cd73c
to
27b5180
Compare
To handle extensions of \Q, we can work over a number field containing all necessary square roots. Currently there is some redundancy. I compute the diagonalizations to know which roots to take, then compute them again when computing Fourier coefficients. |
9b2e77f
to
392a6b9
Compare
getting environment error "pytest is not installed in the venv, skip checking tests that rely on it |
relevant MSE questions: |
dc43776
to
32c6924
Compare
I checked that this builds if merged over the latest beta. Could you fix "This branch is out-of-date with the base branch" ? |
That isn’t necessary until it is fully reviewed and only as a final check with the most recent changes. |
These rebases unbreak the CI - so that's the main reason to do it. |
It looks to me like the CI tests are passing (one skipped) so far. |
Work in issue 38456 is sufficient to construct this DFT. We compute |
So the However, I am thinking we should just reproduce their algorithm directly to take advantage of Sage's linear algebra packages. The only function really used is the What do you think? |
That's unfortunate that it's not included by default. I don't know what the barriers are to getting it incorporated, or how long that would take. It would certainly make things easy. I agree. We'd been talking about doing this anyways, and I'm happy to go ahead and just rewrite it in Sage. I'll try to get a basic version working tomorrow. Then there will probably be ways to optimize it. |
I believe it should just be getting the echelon form of the appropriately augmented Gram matrix of the initial sesquilinear form and then taking the appropriate submatrix (possibly transposed). So it should just be a few lines of code (at most). |
I agree it should a lot simpler than what is written in GAP. However, a straightforward augmentation and row reduction doesn't yield the correct matrix. For example, when
which is correct. However,
which is just
which suggests at some point we should be factoring scalars as |
Okay, a little bit more care is needed here since it does the RREF. We only want to operate on the strictly lower (or upper) triangle of the matrix as the other part is taken care of by the symmetry. So a short version is to use the inverse of the upper part of the LU decomposition:
Of course, this is not very efficient since it is computing far more things than necessary. However, it is easy to code... |
Mathematically it's not echelon form, and not even Gram-Schmid, as you have a twist by the Galois automorphism in the factorisation. |
For what it's worth, here is a direct translation of the GAP code into Sage which appears to be working now. Perhaps we can reduce it, but I agree with Dima that the twist makes it something slightly different.
|
Definitely, I'm surprised I had forgotten to add those. I added two, one for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Some additional formatting things for readability.
5a82f82
to
2e172cc
Compare
Thanks, looks much better. Committed, squashed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, one last mystery to solve. Why do you get a different ordering of the columns than the bots. I don't see where there could be something that produces different orders. Tests are passing locally for you?
Yes, was looking into that. I didn't notice it was the column ordering; I thought they were totally different. I checked that both versions are unitary, so they're passing in that sense. I also compared my local code closely to the Sage code - they seem to match line for line. I initially thought maybe it's computing a different multiplicative generator for some reason. There are probably at least two conjugate square roots. |
Checking to see if the multiplicative generators match, and if the seminormal DFTs match. These are really the only two things it could be. The rest of the computation is just computing conjugate square roots of the diagonal and multiplying by the inverse. |
@tscrim Mystery solved. The multiplicative generators were the same, but I was using |
bc5abc9
to
ae69d72
Compare
I see. That makes sense. The groups ( |
It looks like tests are passing now. Let me know if there's anything else I need to do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I know my comments are coming in small batches over time rather than just being done with it all. This should be the last round though.
Two minor formatting things, plus you should add the "unitary"
input to the SymmetricGroupRepresentations
docstring with an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, now I am doing some obvious optimizations. This will be it from me. Anything else I can do on a followup PR.
No worries. Committed. These are all very helpful additions. Okay, I've added the |
Sounds good. It seems to be in a usable state now, and the documentation is much better. Agreed re: the followup PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Just two last details for the linter.
The bottleneck is now in computing the seminormal form, which seems like a harder problem to optimize (with a fair bit of the time spent in multiplying elements in the SGA).
And some failing tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should address the doctests.
Okay, those minor fixes seem to have doctests passing now. |
check if mult. gen. and seminormal DFT match check to see if the multiplicative generator of GF(25) being computed by Sage matches the one being computed locally. also check the DFT. these are the only two things that may be different. remove double colon in examples I'm using some descriptive strings for each example since they're all a bit different. those sentences have double colons after them, and code immediately follows. Update src/sage/combinat/symmetric_group_algebra.py remove whitespace between `p` and \mid improve doctest examples with better exposition say what the example is before performing it. check that the desired property holds. fix failing doctest examples the computed unitary DFT in Sage is different than when I compute it locally using the same code. not sure why. Update src/sage/matrix/matrix2.pyx fetch the cached rank. if it's full or not yet computed, continue, otherwise throw a ValueError that the matrix is not full rank. better naming convention for SGAs separate the field and the group with an underscore. for fields of size q**2, use the actual squared value remove extraneous ")" chars not sure why these are there. should be GF(9), a field of square order this is meant to test the case when the characteristic divides the order. in this case, p=3, q=9, and n=3. before we were working over GF(3) rather than GF(9), so it triggered the other checks re: whether it is a field of square order. remove unnecessary blank lines, duplicate check remove trailing whitespace update `form` input, add more tests include a description in DFT of the different possible values of `form`. the possible values are `None`, `seminormal`, `modular`, `unitary`. each uses a different basis. also includes new tests for the four different ways that the input field might fail to be a finite field of square order such that the characteristic does not divide the order of the group. Update src/sage/combinat/symmetric_group_algebra.py use lowercase for the ValueError message trigger CI tests use cholesky(extended=True) we have renamed hermitian_decomposition to cholesky(extended=True) Update src/sage/combinat/symmetric_group_representations.py move .H to cached part Update src/sage/combinat/symmetric_group_representations.py move conjugate-transpose to cached part update doctests we're using .H in the unitary_change_of_basis, so the doctests need to reflect that partition, not self._partition forgot underscore, don't define _n define self._n use dimension instead when using the SGA version of specht, we don't have access to the yang_baxter_graph use self._specht with SGA specht module use representation from SGA using self._specht causes tests to fail remove print null_space.basis() appears to be working now with the change to using the symmetric group algebra to get the representation remove print statements use SGA vs. _spect for now include another doctest, print try printing U use A.H instead of A since U = A*A.H, we need to adjust the unitary representation to be A.H\rho(g)A.H.inverse(). also need to update the doctests remove unnecessary import, q, whitespace trigger CI checks use hermitian_decomposition in unitary rep'n we now have a standalone method to compute the Hermitian decomposition of a matrix, so we should use it when computing the change-of-basis matrix for a unitary representation implement unitary DFT of symmetric group over finite fields unitary DFT of symmetric group for number fields and finite fields - perform the unitary DFT of the symmetric group over finite fields and number fields - compute unitary representations of the symmetric group over finite fields - use real orthogonal representations as unitary representations for symmetric group trigger CI checks Update src/sage/matrix/matrix2.pyx add a SEEALSO since this is an extended form of the Cholesky decomposition Update src/sage/matrix/matrix2.pyx remove this break conditional in favor of conditional for while loop Update src/sage/matrix/matrix2.pyx replace always True with explicit conditional move hermitian_decomposition into cholesky this method for decomposition a Hermitian matrix U = AA* is similar in spirit to the Cholesky decomposition, but extends it to work over finite fields of square order. Update src/sage/matrix/matrix2.pyx yes, just forgot to update this initialize row to -1 Update src/sage/matrix/matrix2.pyx add word "package" to GAP ``forms`` Update src/sage/matrix/matrix2.pyx change \ast to * for consistency Trigger CI tests Trigger CI tests Update src/sage/matrix/matrix2.pyx use Matrix handle 1x1 case Update src/sage/matrix/matrix2.pyx cache the rank of the matrix since are computing it anyways add colon after Traceback forgot the colon fail cases should be tests, not examples Update src/sage/matrix/matrix2.pyx add hermitian_decomposition method given a Hermitian matrix U, returns a matrix A such that U = AA* use translated GAP source code for BaseChangeCanonical raise ValueError for non full rank matrices this method does not work for singular matrices. since we are already computing the rank in the row reduction, we can just check at the end whether the rank is full, and if not exit with a ValueError. this may be overcautious - it's not clear that this won't work for some singular matrices. there are cases where it certainly doesn't work, and we include one in the doctest. add example for singular case this example is currently failing and needs to be caught remove unnecessary import, singularity check if the matrix `U` is singular, the process will still result in a matrix but it will fail to have the expected property, namely `B*B.H == U`. Update src/sage/matrix/matrix2.pyx avoid import, directly call `sqrt` method, don't extend to keep result in `ZZ` fix example this was using the output from the `forms` package, when it should be from the GAP source translation. change examples there is a failing example due to the fact that the matrix is singular with q=3, and U = matrix(F,[[1,4,7],[4,1,4],[7,4,1]]). move _cholesky_extended_ff to hidden method move _cholesky_extended_ff to a separate hidden method, and just call it from inside cholesky if extended=true. add doctests and docs for the method separately. sparse for now, should be expanded upon. Update src/sage/matrix/matrix2.pyx add output for GF(3**2) case this case fails to have a lower triangular decomposition which has been checked by brute force. Update src/sage/matrix/matrix2.pyx move two examples over finite fields from tests the two examples computing the extended Cholesky decomposition over square order finite fields are in TESTS, and should be moved to EXAMPLES. Update src/sage/matrix/matrix2.pyx this is better and clarifies that the result of the Cholesky decomposition is lower triangular (and not possibly upper triangular). Update src/sage/matrix/matrix2.pyx add single whitespace before `extended` parameter Update src/sage/matrix/matrix2.pyx looks good. makes sense to point out the Cholesky decomposition might not exist, given that was our initial difficulty. 3 trailing whitespace move hermitian_decomposition into cholesky this method for decomposition a Hermitian matrix U = AA* is similar in spirit to the Cholesky decomposition, but extends it to work over finite fields of square order. add two examples add two examples of a Hermitian decomposition, one of which is not upper/lower triangular (so would be impossible with Cholesky decomposition) Update src/sage/matrix/matrix2.pyx yes, just forgot to update this initialize row to -1 Update src/sage/matrix/matrix2.pyx add word "package" to GAP ``forms`` Update src/sage/matrix/matrix2.pyx change \ast to * for consistency Trigger CI tests use self.__class__ within matrix.pyx file Trigger CI tests remove blank line Update src/sage/matrix/matrix2.pyx Update src/sage/matrix/matrix2.pyx Update src/sage/matrix/matrix2.pyx Update src/sage/matrix/matrix2.pyx Update src/sage/matrix/matrix2.pyx use Matrix handle 1x1 case format failing test case properly format failing test case properly by removing variables that hold results of computation. just call the function that raises the exception Update src/sage/combinat/symmetric_group_representations.py formatting changes (use ^T rather than .T, use `\rho` instead of `rho`). Update src/sage/combinat/symmetric_group_representations.py formatting changes Update src/sage/combinat/symmetric_group_representations.py use `.. MATH::` formatting instead of inline code. Update src/sage/combinat/symmetric_group_representations.py some formatting changes. add a return, and remove "this is". move docstring from __init__ to class level the description of the mathematics should move from the __init__ function which should only contain "Initialize `self`". whitespace, math formatting issues add unitary DFT examples add some examples with `q=5` and `q=7` and `n=3` for the unitary DFT. add unitary DFT example, case for p|n include an example of the unitary DFT under the `dft` method (rather than just _dft_unitary). include a test for a case GF(4)S_3, when 2|3!, so that the unitary DFT throws a NotImplementedError. Update src/sage/combinat/symmetric_group_algebra.py separate each `form` type into three separate bullets, and don't include None remove extra line from debugging add space update doctests with correct outputs it should just say the partition for UnitaryRepresentation. change 3 to 4 since the partition is [3,1] Update src/sage/combinat/symmetric_group_representations.py change 3 to 4 for correct partition size Update src/sage/combinat/symmetric_group_representations.py remove unmatched right parentheses remove whitespace add unitary for SymmetricGroupRepresentation(s) add an input of type `unitary` for the SymmetricGroupRepresentation(s) constructors Update src/sage/combinat/symmetric_group_algebra.py optimization for multiplying the diagonal in the number field case Update src/sage/combinat/symmetric_group_algebra.py Update src/sage/combinat/symmetric_group_algebra.py perform the matrix multiplcation and diagonal matrix multiplcation in the one loop Update src/sage/combinat/symmetric_group_algebra.py replace `\mid` with `|` Update src/sage/combinat/symmetric_group_algebra.py add tick for seminormal Co-Authored-By: Travis Scrimshaw <[email protected]> Co-Authored-By: Dima Pasechnik <[email protected]>
16e1e68
to
bf00417
Compare
This is a positive review from me. |
sagemathgh-38455: unitary DFT for symmetric group algebra <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies - sagemath#39200: Uses the decomposition. <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#38455 Reported by: Jackson Walters Reviewer(s): Jackson Walters, Travis Scrimshaw
📝 Checklist
⌛ Dependencies