Oddities with DirectProductElements

[gwhitney]

This is really three closely related points; I didn't want to clutter the issue list, but let me know if you prefer I break this in three.

1A) Note the following interaction with current master, freshly compiled:

gap> elt := DirectProductElement([1,1]);;
gap> String(elt);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsFinite' on 1 arguments at /home/gwhitney/code/dev/gap/lib/methsel2.g:241 called from
IsFinite( list ) at /home/gwhitney/code/dev/gap/lib/list.gi:298 called from
<function "unknown">( <arguments> )
 called from read-eval loop at *stdin*:2
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue

This would seem to me to be a bug, especially in light of the line in the Reference Manual sec 6.3.2 stating: "One should at the very least install a String (27.7.6) method to allow printing." And since the default PrintString is just to return String, it is clear how these elements should be rendered in String, i.e., as code that GAP would execute to reproduce the element, namely DirectProductElement( [ 1, 1 ] ). This is an easy fix, to add such a String method (which I think would render the existing PrintObj method redundant); I will as soon as I have a chance create a pull request for such a change, unless there's objection here.

1B) That manual section also says "One should usually install a ViewString (27.7.3) method, unless the above String (27.7.6) method is good enough for ViewObj (6.3.5) purposes." I will say that working with DirectProductElements a lot, writing out "DirectProductElement" in every View of such an entity becomes rather repetitive. Often it is better to display information more compactly. So, I'd like to suggest an enhancement of a more compact View of DirectProductElements. Happy to implement in a pull request if there is consensus on the notation. [ 1, 1 ] is out, indistinguishable from a list. {1, 1} is no good, since curly braces are for sets. I think <1, 1> is pretty understandable, and as far as I can see, the only other thing GAP-relevant that angle brackets are used for sometimes is the group generated by some elements. But right now gap shows <group with 2 generators> rather than <g1, g2>, so it seems to me that <1, 1> is available for tuples, and I think is the leading candidate.

Other options: ![1, 1] since they're really internally dense lists, and the ! is sort of a reminder of internal stuff, or <<1, 1>> just to be really distinctive, or a radical one that is surely too big a change to fly: before GAP, I was more used to seeing permutations written as (1 2 3)(4 5), which would free up (1,1) for tuples, but that of course would be a parser change -- so like I said, a pipe dream on my part.

2) If you look at the original error above, it's slightly odd that the ultimate issue is that there is no method for IsFinite. Poking further, we see:

gap> elt := DirectProductElement([1,1]);;
gap> IsFinite(elt);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `IsFinite' on 1 arguments at /home/gwhitney/code/dev/gap/lib/methsel2.g:241 called from
<function "HANDLE_METHOD_NOT_FOUND">( <arguments> )
 called from read-eval loop at *stdin*:3
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> quit;
gap> View(elt);
DirectProductElement( [ 1, 1 ] )
gap> IsFinite(elt);
true

It seemed very odd that simply viewing an element changed its properties. What happens is that inside of ViewObj for a DPE, IsSmallList is called on the DPE, for which there is a method, and then a TrueMethod implication causes the DPE to have the IsFinite property. My question is, does this somewhat wonky behavior suggest any change to the implementation of DPEs? One possibility is that the IsFinite property isn't really meaningful for DPEs, and this aspect of things should simply be ignored. (That's the way I lean.) The second possibility is that IsFinite does mean something for a DPE, namely that it has only finitely many components. This splits into two possiblities-- Is there really no chance at all of a non-finite DPE , in which case we should just mark all DPEs as IsFinite on construction? Or do we really contemplate the possibility of infinite direct products, and so there should be a bona fide IsFinite method on DPEs? Anyhow, happy to implement a change if there's a clear sense that one is needed and what it should be.

[fingolfin]

As to cluttering the issue list: There is no such thing. Indeed, we very much prefer to get one issue filed for each, well, issue. Grouping multiple problems together into a single issue actually complicates things for us, and also leads to mistakes (e.g. an issue describes multiple problems, one gets fixed, the issue gets closed because somebody did not notice there were more problems listed in it).

Case in point: Right now I am not sure whether this has been resolved by your PR #1825 or not. I'll have to read through your long report to figure that out (and even then I might not be able to :-/).

So I am instead asking you: Is this resolved now to your satisfaction? If not, could you succinctly state what you consider the problem? Like: "I do X, then Y happens, but I would expect Z to happen, because REASONS". :-)

Finally, it is quite normal in GAP (and happens quite often) that viewing an object changes it properties, simply because view methods often want to embellish their output with some info about the object being viewed, and that may trigger computations.

[gwhitney]

Yes, I had exactly that concern of crispness/clarity when I posted this; but as a relative newcomer, I was concerned about posting three issues at once. With the slight more experience I have, I will definitely keep future issues focused, and try to be judicious as to which deserve posting.

As for the contents of this issue, such as it is: Item (1A), which was the only cut-and-dried "bug," is definitively resolved by PR #1825.

So next is item (1B), which is an enhancement suggestion: my life interacting with GAP would be notably smoothed if the "View" syntax for a DPE were much less verbose, e.g. <x,y> rather than DirectProductElement( [ x, y ] ). When x and y are <=3 characters each, that's almost a factor of four shorter and can mean the difference of my output scrolling well off the screen vs my being able to take it in at a glance, and it's much easier to focus on the underlying components, which is what one cares about. Plus I think a notation like <x,y> is very recognizable to most mathematicians. However, in all of the interaction with you and other GAP core folks during the work on (1a), no matter how much I hinted, I got no sense of any appetite to revise the View syntax for DPEs, so if your inclination is to close (1B) with a "Won't Implement," I get it. On the other hand, if there's any community uptake of the notion that a shorter View of DPEs would be helpful, then I will issue an (as-perfectly-organized-and-tested-as-I-can) PR implementing that before you can factor the 17th Mersenne prime ;-).

That leaves (2). I get it that in View()-ing an object GAP might well discover more properties about it, e.g., it might find a generating set for a group or a basis for a vector space or whatever, and that makes perfect sense to me. But in this case, one goes from attempting to compute the property IsFinite() being an error, implying that either there isn't a way to to figure out whether a DPE IsFinite or that the semantics of IsFinite are not meaningful/clear on DPEs (which in fact maybe they are not), to the DPE having the IsFinite() property, just by viewing it. And to me, despite growing experience with GAP and the comments above, that is behavior that is "wonky" at best, bordering on buggy, because other computations might fail or succeed depending only on whether I had View()ed a given DPE or not; that just seems confusing/bad behavior. I don't have an issue with the DPE() being in a state of ignorance about IsFinite() when it is created, but if it can figure it out in the course of being View()ed, it should well be able to figure it out in a direct call to IsFinite(). But again, the community consensus of you all may well differ.

So in short (sorry I think I may now have written more than the original report (!) so I haven't really helped matters yet): If I ran the zoo, I would implement ViewString(DirectProductElement([1,1])) to yield <1,1> and I would clarify the semantics of IsFinite() on DPEs thus: IsFinite() is primarily a property of collections (with the exception of floats); the documentation of DPEs is clear that they are to act like intrinsic elements, not collections; and therefore IsFinite() should fail as not applicable to a DPE whether or not it is viewed. But I do not (run the zoo), so you can close this issue if it's clear to you neither of those changes is desirable (or not worth the trouble while moving toward a 4.9 release, and then invite me to open a new issue thereafter); or you can suggest I make a PR for either or both changes and I promptly will.

My apologies about my verbosity, but the answer to your question really depends on the attitude of the community toward points (1B) and (2). Thanks for listening, and I am content with whichever direction you all go.

[fingolfin]

OK, that was long again :-). Regarding (1B): I think it's simply that none of us care about DirectProductElement are printed. I almost never encounter them in my work, and even if, printing them is the least of my concerns. If you'd like them to print differently, make a separate issue on that (ideally with a terse description of the request). Or directly submit a PR implementing the desired change. Since few (if any) other people seem to care, but you do care, that has good chances of being accepted (and at worst, people who object the change will speak up, but then we can have a discussion about the pros and cons).

As to (2): I agree this particular example is a bug (or at least undesirable). But it got lost in this issue, among the other issues you list, and among tons of texts, so I am pretty sure nobody else really has even seen it as a separate issue.

Anyway the solution for that is, for technical reasons, probably is not going to be what you have in mind; rather, IsFinite will have to return true, because DPEs are lists in GAP. You may not like it (I certainly don't!), but it's a fact, and one we cannot easily change, as a change would likely break some code using DPEs, and we try to only do that if there is a really good justification.

So, for now, I submitted PR #1905 to address it that way.

This is similar to the situation for matrices, which right now are lists-of-lists, and therefore IsFinite has to work on them, because you cannot distinguish them from lists. We plan to change that by introducing proper matrix objects, which then won't be lists and IsFinite will error out. But for "old-style" list-of-list-matrices, IsFinite will and must keep working.

[fingolfin]

As far as I can tell, this issue has been resolved (at least in master, did not check the 4.9 branch).

My apologies if I am wrong, and missed something: In that case, please open a new issue (or issues, one for each problem), with a short description of the problem (ideally, a code snippet showing what's wrong, and a description what you'd expect resp. desire instead).

Thanks for taking time to share your feedback with us, we really appreciate it!