Skip to content

Add experimental support for wreath Macdonald polynomials#4797

Merged
fingolfin merged 20 commits intooscar-system:masterfrom
RaphaelPaegelow:master
May 21, 2025
Merged

Add experimental support for wreath Macdonald polynomials#4797
fingolfin merged 20 commits intooscar-system:masterfrom
RaphaelPaegelow:master

Conversation

@RaphaelPaegelow
Copy link
Copy Markdown
Contributor

This PR is adding the computation of wreath Macdonald polynomials to Oscar.

@ulthiel
Copy link
Copy Markdown
Contributor

ulthiel commented Apr 11, 2025

Without having looked at the code at all, this in principle has very much my support!

Ok, I have looked quickly at the code. I think the dependency on Chevie will be a major problem, since this conflicts basically with everything in OSCAR:

import Chevie: complex_reflection_group, charinfo, CharTable, CyclotomicNumbers

In principle this should be possible to "fix" when basing this on my not-finished PR on complex reflection groups (#3342).......

Comment thread src/SymmetricFunctions/wreath_macdonald_polynomials.jl Outdated
@fingolfin
Copy link
Copy Markdown
Member

Thank you for your contribution! One quick note: new code like this should probably go into the "experimental" directory as described at https://docs.oscar-system.org/dev/Experimental/intro/ ; you may also wish to have a look at https://docs.oscar-system.org/dev/DeveloperDocumentation/new_developers/

Comment thread src/SymmetricFunctions/wreath_macdonald_polynomials.jl Outdated
@joschmitt joschmitt added the experimental Only changes experimental parts of the code label Apr 14, 2025
Copy link
Copy Markdown
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments that should hopefully help you a bit with the experimental structure, and to get the code running again

Comment thread experimental/WreathMacdonaldpols/src/wreath_macdonald_polynomials.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/wreath_macdonald_polynomials.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/wreath_macdonald_polynomials.jl Outdated
Comment thread experimental/WreathMacdonaldpols/docs/doc.main
Comment thread src/Oscar.jl Outdated
Comment thread src/Oscar.jl
Comment thread src/exports.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/wreath_macdonald_polynomials.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Copy link
Copy Markdown
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great progress. I found some few julia performance pitfalls that should be pretty easy to adapt here

Comment thread Project.toml Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
@RaphaelPaegelow
Copy link
Copy Markdown
Contributor Author

I have removed Chevie as a dependency thanks to #4821.
I will now improve readability of the output of wreath_macs.

Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/README.md Outdated
Comment thread experimental/WreathMacdonaldpols/README.md Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Copy link
Copy Markdown
Member

@lgoettgens lgoettgens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two small typos in docstrings that don't match the latest changes. apart from that, I have no objections to merging this (although have haven't checked the mathematics)

Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
@lgoettgens lgoettgens requested review from fingolfin and ulthiel May 19, 2025 14:00
@ulthiel
Copy link
Copy Markdown
Contributor

ulthiel commented May 19, 2025

Sorry, can someone show me a screenshot/pdf whatever of the documentation? I forgot how to pull this.

@fingolfin
Copy link
Copy Markdown
Member

@ulthiel easiest to "pull this" might be to install gh (see https://cli.github.com) and then just do gh pr checkout 4797 in your Oscar clone).

But I did it for you and attach a PDF export of the relevant manual section.
Wreath Macdonald polynomials · Oscar.pdf

Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl
Copy link
Copy Markdown
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, I am sorry, but here are yet some more comments -- but we could also do them later. But at least the typo fixes etc. hopefully cause no harm.

I'll approve anyway because I think it's fine even as it is, just could be even better ;-).

Might be nice to wait a little bit in case @ulthiel has mathematical feedback

r::Int,
wperm::PermGroupElem,
coroot::Vector{Int};
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kwarg type is overly specific, this should be enough

Suggested change
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})
parent::MPolyRing{<:QQAbFieldElem})

r::Int,
wperm::PermGroupElem,
coroot::Vector{Int};
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's provide a default parent for the user's convenience:

Suggested change
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})
parent::MPolyRing{<:QQAbFieldElem} = polynomial_ring(K,[:q,:t];cached=false))

Actually I think we have allowed this to be a cached ring for added convenience in other cases? @thofma ?

Suggested change
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})
parent::MPolyRing{<:QQAbFieldElem} = polynomial_ring(K,[:q,:t];cached=true))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't polynomial_ring returning a tuple?
By default I can cache the ring if that is more convenient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to parent::MPolyRing{<:QQAbFieldElem} = polynomial_ring(abelian_closure(QQ)[1],[:q,:t];cached=true)[1]).

coroot::Vector{Int};
parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})

Given two integers n and r and an element of the affine Weyl group of type A (seen as
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Given two integers n and r and an element of the affine Weyl group of type A (seen as
Given two integers `n` and `r` and an element of the affine Weyl group of type A (seen as

Given two integers n and r and an element of the affine Weyl group of type A (seen as
the semi-direct product of the Symmetric group with the coroot lattice), this function
returns the square matrix of coefficients of the wreath Macdonald polynomials associated with
all multipartition of size n and length r in the standard Schur basis indexed by the
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
all multipartition of size n and length r in the standard Schur basis indexed by the
all multipartition of size `n` and length `r` in the standard Schur basis indexed by the

parent::AbstractAlgebra.Generic.MPolyRing{QQAbFieldElem{AbsSimpleNumFieldElem}})

Given two integers n and r and an element of the affine Weyl group of type A (seen as
the semi-direct product of the Symmetric group with the coroot lattice), this function
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the semi-direct product of the Symmetric group with the coroot lattice), this function
the semi-direct product of the symmetric group with the coroot lattice), this function

You write "the" symmetric group -- but there are many? Perhaps "the symmetric group of degree XYZ" and then replace XYZ with a suitable value. Otherwise, "a" symmetric group?

Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
@ulthiel
Copy link
Copy Markdown
Contributor

ulthiel commented May 20, 2025

I have some comments on the documentation, see attached.
Wreath.Macdonald.polynomials.Oscar.pdf

@RaphaelPaegelow
Copy link
Copy Markdown
Contributor Author

Ahhh, I am sorry, but here are yet some more comments -- but we could also do them later. But at least the typo fixes etc. hopefully cause no harm.

I'll approve anyway because I think it's fine even as it is, just could be even better ;-).

Might be nice to wait a little bit in case @ulthiel has mathematical feedback

Thanks a lot for all your comments.

@RaphaelPaegelow
Copy link
Copy Markdown
Contributor Author

I have some comments on the documentation, see attached. Wreath.Macdonald.polynomials.Oscar.pdf

Thank you very much for your comments. I have implemented your recommendations and have reorganized a bit the documentation. I just don't exactly know which definitions I should add. Could you precise what you mean? I started to write something but quickly realized that I was summarizing Orr's and Shimozono's survey.

@ulthiel
Copy link
Copy Markdown
Contributor

ulthiel commented May 20, 2025

I have some comments on the documentation, see attached. Wreath.Macdonald.polynomials.Oscar.pdf

Thank you very much for your comments. I have implemented your recommendations and have reorganized a bit the documentation. I just don't exactly know which definitions I should add. Could you precise what you mean? I started to write something but quickly realized that I was summarizing Orr's and Shimozono's survey.

I (maybe just me) think it would be nice if the documentation would be somewhat self-contained so that I do not have to go and read a paper to find somewhere in it the definition of the objects the functions return. It must be possible to summarize this in a couple of paragraphs.

Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
Comment thread experimental/WreathMacdonaldpols/src/WreathMacdonaldpols.jl Outdated
@fingolfin
Copy link
Copy Markdown
Member

While agree with @ulthiel that "somewhat self-contained documentation" would be preferable, let's not forget that this Pr targets experimental. So it is perfectly fine to merge it now and make those improvements later.

I've fixed two minor issues directly now, if tests pass, let's merge this, and hopefully @RaphaelPaegelow will consider tweaking the docs in a follow-up PR.

@fingolfin fingolfin enabled auto-merge (squash) May 21, 2025 13:34
@fingolfin fingolfin added enhancement New feature or request release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels May 21, 2025
@fingolfin fingolfin changed the title wreath Macdonald polynomials Add experimental support for wreath Macdonald polynomials May 21, 2025
@fingolfin fingolfin merged commit c9565f6 into oscar-system:master May 21, 2025
32 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request experimental Only changes experimental parts of the code release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants