Skip to content
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

Hack to "improve" (?) printing of vectors of ring/field/module elements #2064

Closed
fingolfin opened this issue Mar 16, 2023 · 2 comments
Closed

Comments

@fingolfin
Copy link
Member

fingolfin commented Mar 16, 2023

summary(io::IO, a::AbstractArray{<:RingElem}) = oscar_array_summary(io, a, axes(a))
function oscar_array_summary(io::IO, a, inds::Tuple{Vararg{OneTo}})
    print(io, Base.dims2string(length.(inds)), " ")
    showarg(io, a, true)
end

Before:

julia> v
2-element Vector{AbstractAlgebra.Generic.MPoly{nf_elem}}:
 x^2 - 3*y
 1//4*x^2 - 1//2*sqrt(3)*x*y + 3//2*sqrt(3)*x + 3//4*y^2 + 3//2*y

Apply this hack (?):

function Base.showarg(io::IO, x::Vector{<:RingElem}, toplevel::Bool)
    if toplevel
      print(io, "Vector of ring elements")
      return
    end
    # original code follows
    toplevel || print(io, "::")
    print(io, typeof(x))
end

After:

julia> v
2-element Vector of ring elements:
 x^2 - 3*y
 1//4*x^2 - 1//2*sqrt(3)*x*y + 3//2*sqrt(3)*x + 3//4*y^2 + 3//2*y

Of course one could do something similar for FieldElem, ModuleElem, etc.
and the output could be refined, e.g. it could print the parent (though I'd
not do this for now, as it raises many questions: handle empty vectors; should
it check that all entries have the same parent; and if not, what then? etc.)

Questions:

  1. Is this type piracy? Vector is not our type, but RingElem is...
  2. Would this be useful for improving our printing situation?
  3. Where else is using showarg? If the above is too broad, we could also
    instead override summary(io::IO, x::Vector{<:RingElem}).
@fingolfin fingolfin added this to the OSCAR 0.12.0 milestone Mar 16, 2023
@thofma
Copy link
Collaborator

thofma commented Mar 16, 2023

I understand the reason why one would one want this.

  1. I always had the impression, that this is something hacky and could break julia internals in some inpredictable way? See Consider deleting custom show methods for types PainterQubits/Unitful.jl#321, Redefining show for Type{Foo{T}} breaks display of any MethodError JuliaLang/julia#13306, Remove show for Type{<:Pullback} FluxML/Zygote.jl#1356. I know it is a bit different, but maybe the same problems could arise in theory?
  2. How do I print the the type of the vector? @show typeof(x) will not work anymore.
  3. It makes debugging much harder if someone just gives me some output of a session.

@simonbrandhorst
Copy link
Collaborator

@HechtiDerLachs
At some point in the future something like this might help with the very long types in schemes.

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

No branches or pull requests

3 participants