Clarify the relation between `supercompact` and `compact` for printing

[fingolfin]

Right now these two coexist and it is not quite clear how they are supposed to interact.

This leads to changes like https://github.com/Nemocas/AbstractAlgebra.jl/pull/1627 which just uses both. Should we do this globally? Or should perhaps supercompact imply compact (the former is "our" setting, the latter is Julia's)? Or something else

CC @simonbrandhorst @thofma @fieker

[fingolfin]

It would be great to discuss this with @simonbrandhorst who made the original plan for the revised printing -- Simon, let us know when you have time (or perhaps you can make it to one of the regular Wednesday online meetings / coding sprints)

[simonbrandhorst]

:supercompact is ours and :compact is julia's. As far as I can tell there are no implications. quoting from the manual " The following properties are in common use:

:compact: Boolean specifying that values should be printed more compactly, e.g. that numbers should be printed with fewer digits. This is set when printing array elements. :compact output should not contain line breaks.

"

Apparently it is used when printing multidimensional arrays but not when printing Vector. If you care about how multidimensional arrays print you can set this option. Probably it is most relevant for element types.

[fingolfin]

We discussed this at length during the Wednesday coding sprint. Overall we found it very confusing that supercompact is supposed to be completely independent from compact given that name which strongly suggests an implication.

If they are ultimately really meant to be independent, we think it should be renamed. Moreover, this relation should still be documented explicitly.

But before we do any that, we feel we need to carefully study and revise how this (and the rest of the printing system) works anyway. We felt that there are already a bunch of things that could and should be tweaked in the system. E.g. @show_name and @show_special should be advertised on the printing dev docs, and we should have a push to have them be used more

In particular we looked at existing code using supercompact and compact. We tried to find examples where show methods treated compact and supercompact differently, and where we thought this made sense. We didn't find any, but rather in many cases we determined that printing would have been nicer if the supercompact code had been triggered by compact being enabled (or vice-versa for older printing code that predates supercompact but did handle compact).

Overall we were then wondering whether it really makes sense to keep supercompact and compact separate. A bunch of current issues or enhancement requests would be satisfied if we just treated them as equal everywhere. Of course keeping then separate would at least allow us to treat them different in the future... Perhaps @simonbrandhorst or @thofma can think if examples where it would be beneficial?

In the meantime, we formulated the following action plan for the immediate future:

For now we will experiment with essentially

We add a few helper methods to AA:

# convenient way to request supercompact printing; easier to remember,
# and also protects against typos.
# moreover, for the time being this enables supercompact and compact together;
# once everything uses the new helpers we can then easily experiment
# with either merging supercompact and compact; or again separating them
supercompact(io::IO) = IOContext(io, :compact => true, :supercompact => true)

# helper to test if supercompact mode is on (easier to remember, type stable,
# and also protects against typos
is_supercompact(io::IO) = get(io, :supercompact, false)::Bool

# helper to test if compact mode is on (easier to remember, type stable,
# and also protects against typos
# For now, also enforces implication 
is_compact(io::IO) = get(io, :compact, false)::Bool || is_supercompact(io)

Then a typical show method for a parent object type would look like this:

function show(io::IO, R::SomeRing)
   @show_name(io, R)
   @show_special(io, R)
   if is_compact(io)
      # no nested printing
      ...
   else
      # nested printing allowed, preferably supercompact
      ...
   end
end

After doing this "everywhere", compact and supercompact would be effectively equivalent. If we end up deciding we want to merge them it'll be easy enough to implement using some search & replace. If we determine we want to keep them separate we can get rid of the implication in one or both of is_compact and supercompact and proceed accordingly.

Assuming @simonbrandhorst and @thofma are also OK with this experiment, we would gradually go through AA, Nemo, Hecke, Oscar to update that the existing show methods for all kinds of ring and ring element types to use the @show_ macros and new helpers.

[thofma]

I don't quite follow all the reasons why something must change. But let me mention some things so that we are all on the same page.

1. There is a lot of code out there, which predates the "new printing system". And in the past, we were not quite sure what :compact was about. Changes are high that we misused things. The code which sets :compact => true should just be removed. It will lead to unintended side effects.
2. :compact is some property that julia "randomly" adds to IO object, e.g. when printing a::Array{T, N} with N > 1.
3. The whole printing system that we cooked up was only meant for parent objects (as far as I remember). So unless someone was printing a 2-dimensional array of rings or groups, one should never encounter :compact. For elements, we mainly do the expressify route, which has its own handling of :compact etc.
4. It is not clear if julia's :compact should trigger our :supercompact or our "one-line". This is mainly about the question of whether you want

julia> [Qx Qx; Qx Qx]
2×2 Matrix{QQMPolyRing}:
 Multivariate polynomial ring in 2 variables over QQ  Multivariate polynomial ring in 2 variables over QQ
 Multivariate polynomial ring in 2 variables over QQ  Multivariate polynomial ring in 2 variables over QQ

or

julia> [Qx Qx; Qx Qx]
2×2 Matrix{QQMPolyRing}:
 Multivariate polynomial ring  Multivariate polynomial ring
 Multivariate polynomial ring  Multivariate polynomial ring

The first example is the current state of the art, since :compact implies "one-line", at least for Array printing. And, how important is changing this for us? Or do we even want this?

As far as I can see, we are mainly irritated by the word :supercompact. So here is my proposal:

Rename :supercompact it to :terse_printing_for_nested_printing.

Edit: In case it is not clear, I don't see the value of the experiment, since I don't understand why something must change except for the name.

[fingolfin]

First off, I'd be happy to do something else, that addresses the various issues that float around printing.

IMHO terse_printing_for_nested_printing is too long but we discuss minimal and nonrecursive.

But something must change.. As you say: a lot of our printing code still predates the new concept and as such is not honoring supercompact at all, but is instead honoring compact ; and other code setting compact when it maybe really should be setting supercompact -- but thanks to the lack of explantory comments, the intent of the code is often unclear.

In particular some code already sets compact in one places and checks for compact in another, which "fit together". If we change compact to supercompact in one but not in the other, stuff starts breaking. If we don't change anything, then supercompact doesn't work. So we can then either...

• modify the show method to also handle supercompact
• modify the code calling show to set supercompact and compact (maximizes compatibility)
• modify the code calling show to set supercompact but not compact (this works as long as the printing code it calls recursively already was updated to handle supercompact

Here is a random example from Hecke.jl that handles compact but not compact (and also is missing oneline vs. detailed handling).

function show(io::IO, O::AlgAssRelOrd)
  compact = get(io, :compact, false)
  if compact
    print(io, "Order of ")
    show(IOContext(io, :compact => true), algebra(O))
  else
    print(io, "Order of ")
    println(io, algebra(O))
    print(io, "with pseudo-basis ")
    pb = pseudo_basis(O, copy = false)
    for i = 1:degree(O)
      print(io, "\n(")
      show(IOContext(io, :compact => true), pb[i][1])
      print(io, ", ")
      show(IOContext(io, :compact => true), pb[i][2])
      print(io, ")")
    end
  end
end

For the sake of not talking past each other, perhaps you can suggest how you'd envision how we'd handle this specific example, @thofma ?

BTW, I think the supercompact(io) and is_supercompact (or perhaps supercompact_enabled() or whatever) helpers have independent value for the reason I mentioned: detecting typos, can be tab completed, (and IMHO easier to remember, but that's subjective)

The other issue (which is not in the title of this one, but that we discussed) is the inconsistent use of @show_name and @show_special. We kind of agreed that it doesn't make sense to use this for some rings but not others. So we'd like to insert this into all parent show methods.

[fingolfin]

Of course https://github.com/Nemocas/AbstractAlgebra.jl/pull/1627 from the issue description is also relevant; I'd appreciate comments on it, too, @simonbrandhorst @thofma

[thofma]

The origin of all the :compact business is https://github.com/Nemocas/AbstractAlgebra.jl/pull/294. I hope this makes the original intent of using :compact clearer. Since then, I would say we learned two things:

1. The applications, in particular in Oscar, show that we need something more fine-grained.
2. The printing system in julia is not very "implementation friendly" (I won't go into details). In particular in which situation which function is called with which arguments. Thus one should at all costs avoid relying on :compact.

From this point of view, all the :compact things in AA/Nemo(?)/Hecke are technical debt and should be removed. This brings us to

• modify the show method to also handle supercompact
• modify the code calling show to set supercompact and compact (maximizes compatibility)
• modify the code calling show to set supercompact but not compact (this works as long as the printing code it calls recursively already was updated to handle supercompact

I would prefer the last option. There are already some changes lined up for breaking the printing in AA, so we would not need to worry about compability.

Here is a random example from Hecke.jl that handles compact but not compact (and also is missing oneline vs. detailed handling).

[...]

For the sake of not talking past each other, perhaps you can suggest how you'd envision how we'd handle this specific example, @thofma ?

This is one of the many examples, which we did not handle before Oscar 1.0 because of time constraints. After implementing the "new printing" for the pi[i][2] (which are ideals), this would become

function Base.show(io::IO, O::AlgAssRelOrd)
  if get(io, :supercompact, false)
    # no nested printing
    print(io, "Order")
  else
    print(io, "Order of ")
    print(IOContext(io, :supercompact => true), algebra(O))
  end
end

function Base.show(io::IO, ::MIME"text/plain", O::AlgAssRelOrd)
    println(io, "Order of ")
    println(io, algebra(O))
    print(io, "with pseudo-basis ")
    pb = pseudo_basis(O, copy = false)
    for i = 1:degree(O)
      print(io, "\n(")
      show(io, pb[i][1])
      print(io, ", ")
      show(Io, pb[i][2])
      print(io, ")")
    end
  end
end

(I am ignoring indentation and some other details)

BTW, I think the supercompact(io) and is_supercompact (or perhaps supercompact_enabled() or whatever) helpers have independent value for the reason I mentioned: detecting typos, can be tab completed, (and IMHO easier to remember, but that's subjective)

Agreed.

I think figuring out if and when we should honor a :compact => true, which after the proposed changes above can only come from julia itself, is a different question. We should first make all things consistently use :supercompact or whatever the name is.

[fingolfin]

Just to record this here explicitly: I am happy with the plan as outlined with @thofma

[fingolfin]

This should be resolved as we essentially renamed supercompact to terse