Skip to content

Add multicombinations to Oscar#5477

Merged
fingolfin merged 10 commits intooscar-system:masterfrom
mjrodgers:multicombinations
Nov 5, 2025
Merged

Add multicombinations to Oscar#5477
fingolfin merged 10 commits intooscar-system:masterfrom
mjrodgers:multicombinations

Conversation

@mjrodgers
Copy link
Copy Markdown
Collaborator

The UI directly mirrors that of combinations, the only difference being that we get combinations with repetitions allowed.

At the moment I still need to add some tests and include documentation in the Oscar manual. But we see a nice speedup compared to the version in Experimental/LieAlgebras (which to be fair was never claiming to be optimized at all).

julia> @b collect(Oscar.multicombinations(20,7))
17.005 ms (657803 allocs: 75.279 MiB)

julia> @b collect(Oscar.LieAlgebras.multicombinations(20,7))
260.422 ms (1317337 allocs: 515.105 MiB, 31.65% gc time, without a warmup)

@mjrodgers mjrodgers added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Oct 20, 2025
@mjrodgers mjrodgers requested review from fingolfin and thofma October 20, 2025 13:22
@lgoettgens
Copy link
Copy Markdown
Member

Could you also please remove the implementation in Experimental/LieAlgebras and redirect all call-sites to this new implementation?

@fingolfin fingolfin closed this Oct 28, 2025
@fingolfin fingolfin reopened this Oct 28, 2025
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.

Great!

@fingolfin
Copy link
Copy Markdown
Member

This should be hooked up into the manual, too!

We can of course merge this now and do that in a follow-up PR, but ideally that should come soon so we don't forget about it.

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 29, 2025

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.19%. Comparing base (24341a3) to head (be6453b).
⚠️ Report is 19 commits behind head on master.

Files with missing lines Patch % Lines
...rics/EnumerativeCombinatorics/multicombinations.jl 92.30% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5477      +/-   ##
==========================================
+ Coverage   84.05%   84.19%   +0.13%     
==========================================
  Files         730      731       +1     
  Lines       99233   101201    +1968     
==========================================
+ Hits        83412    85206    +1794     
- Misses      15821    15995     +174     
Files with missing lines Coverage Δ
experimental/LieAlgebras/src/Combinatorics.jl 100.00% <ø> (ø)
experimental/LieAlgebras/src/LieAlgebraModule.jl 90.23% <100.00%> (ø)
...xperimental/LieAlgebras/test/Combinatorics-test.jl 92.30% <ø> (+0.64%) ⬆️
...rc/Combinatorics/EnumerativeCombinatorics/types.jl 100.00% <100.00%> (ø)
...rics/EnumerativeCombinatorics/multicombinations.jl 92.30% <92.30%> (ø)

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread test/Combinatorics/EnumerativeCombinatorics/multicombinations.jl
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
@fingolfin fingolfin enabled auto-merge (squash) November 5, 2025 13:25
@fingolfin fingolfin merged commit b866fe3 into oscar-system:master Nov 5, 2025
39 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 release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: combinatorics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants