Add fixed_points for PermGroup and PermGroupElem

[varuntrehan7]

I was able to write a docstring in the groups/perm.jl and I was also able to write some tests in the tests/ groups.

I was successfully able to add and validate tests for the fixed points in symmetric group but while trying to do it with trivial group or subgroups generated by specific cycles I was facing some issues.

[fieker]

your commit is too large, changes to Prohejt.toml should not be there.
Also, as you see here, the tests are failing...

[varuntrehan7]

Yes, I'll rectify them immediately. Thankyou.

…

On Fri, Aug 29, 2025 at 11:20 Claus Fieker ***@***.***> wrote: *fieker* left a comment (oscar-system/Oscar.jl#5250) <#5250 (comment)> your commit is too large, changes to Prohejt.toml should not be there. Also, as you see here, the tests are failing... — Reply to this email directly, view it on GitHub <#5250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BUWKEAP5LJBLAX4XFRCVKST3QALMZAVCNFSM6AAAAACFD4YYJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMZWGM2DSMRQG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[fieker]

You still have changes to Project.toml in

[varuntrehan7]

You still have changes to Project.toml in

I think I was able to rectify that , next I will look into the errors that are coming up.

[lgoettgens]

Thanks for your contribution! I added some more comments regarding code style. The Project.toml changes and the unnecessary testfile are definitely blocking this PR from getting merged (as are the failing test).

[lgoettgens]

Suggested change

	fixed_points(G::PermGroup) = setdiff!(collect(1:degree(G)), moved_points(G))
	fixed_points(G::PermGroup) = setdiff(1:degree(G), moved_points(G))

I did a small benchmark and it seems as if the original version of this was indeed better than my suggetsion, so this should be changed back.

julia> f1(n, x) = setdiff(1:n, x)
f1 (generic function with 1 method)

julia> f2(n, x) = setdiff(collect(1:n), x)
f2 (generic function with 1 method)

julia> f3(n, x) = setdiff!(1:n, x)
f3 (generic function with 1 method)

julia> f4(n, x) = setdiff!(collect(1:n), x)
f4 (generic function with 1 method)

julia> @b f1($(10), $([1,3,5,6]))
382.658 ns (6 allocs: 448 bytes)

julia> @b f2($(10), $([1,3,5,6]))
416.382 ns (8 allocs: 592 bytes)

julia> @b f3($(10), $([1,3,5,6]))
ERROR: CanonicalIndexError: setindex! not defined for UnitRange{Int64}
[...]

julia> @b f4($(10), $([1,3,5,6]))
398.235 ns (7 allocs: 544 bytes)

[lgoettgens]

Try to add fixed_points to

Oscar.jl/experimental/IntersectionTheory/src/IntersectionTheory.jl

Line 7 in 013858b

import ..Oscar: pullback, pushforward, base, OO, product, compose, identity_map, map

this should then get rid of the issues related to intersection theory that you encounter

[varuntrehan7]

Try to add fixed_points to

Oscar.jl/experimental/IntersectionTheory/src/IntersectionTheory.jl

Line 7 in 013858b

import ..Oscar: pullback, pushforward, base, OO, product, compose, identity_map, map

this should then get rid of the issues related to intersection theory that you encounter

Thanks, that's helpful.

[fingolfin]

@varuntrehan7 please also address the other comments @lgoettgens made

[fingolfin]

@varuntrehan7 that means: look over all of #5250 and https://github.com/oscar-system/Oscar.jl/pull/5250/files for the code change suggestions -- you can either apply this with a few clicks via the webinterface (and later use git pull to get them onto your computer) or you need to manually replicate them on your computer and push your changes.

[varuntrehan7]

@varuntrehan7 that means: look over all of #5250 and https://github.com/oscar-system/Oscar.jl/pull/5250/files for the code change suggestions -- you can either apply this with a few clicks via the webinterface (and later use git pull to get them onto your computer) or you need to manually replicate them on your computer and push your changes.

Thankyou, yes I'll make sure I do all the suggestions made so far and push again after doing so. apologies for the delay, I will do it asap.

[fingolfin]

I am getting a bit tired of repeating this, but here it is one more time: number_of_fixed_points is still missing.

[fingolfin]

Thanks, it's starting to look a lot better.

[fingolfin]

Please go everything and remove any unicode. Otherwise this seems OK now

[lgoettgens]

@varuntrehan7 If you think this is ready, please mark it as "ready to review" using the github UI.
However, some tests still seem to be failing...

[lgoettgens]

Suggested change

	Return the number of those points in `1:degree(x)` or `1:degree(G)`,
	respectively, that are fixed under the action `^`.
	Return the number of points in `1:degree(x)` or `1:degree(G)` that are
	fixed by all elements (for groups) or by `x` (for elements).

I would find it easier to understand if both docstrings use the same sentence structure

[varuntrehan7]

@varuntrehan7 If you think this is ready, please mark it as "ready to review" using the github UI. However, some tests still seem to be failing...

yeah, i'm not sure they were not failing before. Let me rectify them and and then mark it "ready for review".

[lgoettgens]

Suggested change

	@test number_of_fixed_points(z) == 0
	@test number_of_fixed_points(z) == 1

as you write in line 463, there is a 1-cycle containing 4, and thus 4 is a fixed point.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.99%. Comparing base (1738e00) to head (6fccfd4).
⚠️ Report is 104 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5250      +/-   ##
==========================================
- Coverage   84.91%   83.99%   -0.92%     
==========================================
  Files         713      721       +8     
  Lines       96340    98483    +2143     
==========================================
+ Hits        81806    82720     +914     
- Misses      14534    15763    +1229     

Files with missing lines	Coverage Δ
...ental/IntersectionTheory/src/IntersectionTheory.jl	100.00% <ø> (ø)
src/Groups/perm.jl	95.26% <100.00%> (-0.84%)	⬇️

... and 167 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lgoettgens]

No more objections from my side