Skip to content

Update show for GroupAlgebraElem with sparse rep#1688

Open
lgoettgens wants to merge 2 commits intothofma:masterfrom
lgoettgens:lg/groupalg-elem-show
Open

Update show for GroupAlgebraElem with sparse rep#1688
lgoettgens wants to merge 2 commits intothofma:masterfrom
lgoettgens:lg/groupalg-elem-show

Conversation

@lgoettgens
Copy link
Copy Markdown
Contributor

Let me show the current issue with an example. To reproduce, you need the branch https://github.com/oscar-system/Oscar.jl/tree/lg/character-ring together with a dev'ed Hecke that includes #1685.

julia> R = root_system(:A, 2);

julia> P = weight_lattice(R); # this is basically ZZ^2, but with nice printing

julia> ZP = group_algebra(ZZ, P);

julia> z = zero(ZP)
0

julia> o = one(ZP)
0

julia> z == o # but they print the same
false

julia> 42 * one(ZP) # that should be something like `42 * 0` or `42 * e(0)`
0

julia> a = ZP(gen(P, 1)) + ZP(gen(P, 2))
w_1 + w_2

julia> b = ZP(gen(P, 1) + gen(P, 2))
w_1 + w_2

julia> a == b # but they print the same
false

Thus I propose to instead of just writing the group elem as the basis elem, to do something like e(g) as often done in the literature. With this change enabled, the problematic lines from above change to the following, which IMO is a clear readability and disambiguation improvement.

julia> z = zero(ZP)
0

julia> o = one(ZP)
e(0)

julia> 42 * one(ZP) # that should be something like `42 * 0` or `42 * e(0)`
42*e(0)

julia> a = ZP(gen(P, 1)) + ZP(gen(P, 2))
e(w_1) + e(w_2)

julia> b = ZP(gen(P, 1) + gen(P, 2))
e(w_1 + w_2)

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 19, 2024

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.95%. Comparing base (1b69e3d) to head (b3bec6b).
⚠️ Report is 490 commits behind head on master.

Files with missing lines Patch % Lines
src/AlgAss/Elem.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1688      +/-   ##
==========================================
- Coverage   75.96%   75.95%   -0.01%     
==========================================
  Files         362      362              
  Lines      114325   114325              
==========================================
- Hits        86846    86840       -6     
- Misses      27479    27485       +6     
Files with missing lines Coverage Δ
src/AlgAss/Elem.jl 88.35% <50.00%> (ø)

... and 28 files with indirect coverage changes

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

@fingolfin
Copy link
Copy Markdown
Contributor

Seems sensible to me

@fingolfin
Copy link
Copy Markdown
Contributor

@thofma any thoughts on this PR?

@thofma
Copy link
Copy Markdown
Owner

thofma commented Feb 25, 2025

Have not made up my mind yet unfortunately.

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

Successfully merging this pull request may close these issues.

3 participants