Remove prim and trans folders

[hulpke]

With the primitive and transitive groups libraries moving into required packages:

https://github.com/gap-packages/primgrp

respectively

https://github.com/hulpke/transgrp

we need to set a timeline when the corresponding folders in the main directory will go away. This issue is created to make others aware of it.

As of Dec. 2016, the status is:

• Both libraries exist in package form, providing functionality equivalent to 4.8
• If the packages are available, the Master branch will not load the trans/prim folders

concerning transgrp, there is a working 1.0 package wrapping of transgrp. I hope to have a release including degrees up to 47 in the foreseeable future.

I have tentatively set a 4.9 milestone, please feel free to delay it if it seems appropriate.

[fingolfin]

Both packages are incompatible with browse. Loading GAP when transpkg is present and Browse is autoloading produces this:

Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/Browse/app/transbrowse.g:53
  if not IsBound( TRANSPROPERTIES[ id[1] ] ) or
                                 ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/Browse/app/transbrowse.g:54
     not IsBound( TRANSPROPERTIES[ id[1] ][ id[2] ] ) then
                                 ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/Browse/app/transbrowse.g:55
    TransGrpLoad( id[1], id[2] );
                ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/Browse/app/transbrowse.g:57
  return TRANSPROPERTIES[ id[1] ][ id[2] ];
                        ^
Error, Variable: 'TRANSDEGREES' must have a value
not in any function at /Users/mhorn/Projekte/GAP/gap.github/pkg/Browse/app/transbrowse.g:60

For primpkg, I get this:

Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1555
    for i in [ 1 ..  PRIMRANGE[ Length( PRIMRANGE ) ] ] do
                              ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1555
    for i in [ 1 ..  PRIMRANGE[ Length( PRIMRANGE ) ] ] do
                                                  ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1643
      if deg > PRIMRANGE[ Length( PRIMRANGE ) ]
                        ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1643
      if deg > PRIMRANGE[ Length( PRIMRANGE ) ]
                                            ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1652
      cand:= AllPrimitiveGroups(
                               ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1662
      cand:= AllPrimitiveGroups(
                               ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1664
                 NrMovedPoints, [ 1 .. PRIMRANGE[ Length( PRIMRANGE ) ] ],
                                                ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:1664
                 NrMovedPoints, [ 1 .. PRIMRANGE[ Length( PRIMRANGE ) ] ],
                                                                    ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:2041
            if deg <= PRIMRANGE[ Length( PRIMRANGE ) ] then
                               ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:2041
            if deg <= PRIMRANGE[ Length( PRIMRANGE ) ] then
                                                   ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:2067
            if deg <= PRIMRANGE[ Length( PRIMRANGE ) ]
                               ^
Syntax warning: Unbound global variable in /Users/mhorn/Projekte/GAP/gap.github/pkg/ctbllib/gap4/ctdbattr.g:2067
            if deg <= PRIMRANGE[ Length( PRIMRANGE ) ]
                                                   ^

Of course it is a bit nasty that browse directly accesses the undocumented innards of both libraries. But I hope this can be fixed, possibly after adding some suitable official interfaces to the required data to primpkg and transpkg.

Perhaps @frankluebeck would like to comment? Note that there are several pending patches for Browse already, so a new Browse release would be nice to have anyway.

[olexandr-konovalov]

It is absolutely correct to set 4.9 milestone, and we're not miles away from it, indeed: there seem to be not much work left in SmallGrp and PrimGrp, and thanks to @hulpke with TransGrp too.

@fingolfin, I know too about this problem with Browse. But if we plan to make these three packages needed for GAP for backwards compatibility (what, of course, can be done for each of them only as soon as there will be a proper release of the respective package), then we will be able to provide an updated set of packages with the beta release, and then packages like Browse may decide what to do - introduce new dependency, for example.

[fingolfin]

@alex-konovalov OK, adding explicit dependencies to browse on primpkg and transpkg might be an easy solution. But it requires good coordination with the browse authors (have you heard back from them?), and also it does not solve the problem that browse accesses undocumented internals of the transitive groups database. Additionally, it seems to contain a lot of explicit documentation about some of these otherwise undocumented internals... thus effectively documenting them... I just wonder whether (a) some of the documentation should be moved to the transpkg manual, and (b) whether new documented APIs could/should be added to transpkg so that Browse does not have to rely on undocumented ones anymore...

[fingolfin]

Existing issues relevant to this are #791 and #792.

[fingolfin]

@alex-konovalov OK, thanks for pointing me to / reminding me of #791 and #792. The discussion on the latter has grown quite long, it seems... But I see now that @frankluebeck did indeed reply (and I also participated in the discussion... ouch... forgot all about that :/), and that the plan is to make the new package "needed" by GAP, at least temporarily, until packages with adjusted explicit deps are released.

That still leaves me wondering: Who is going to do the work? And when will it happen? Shouldn't we ASAP remove trans and prim, and make transpkg and primpkg required? Of course we'd have to warn devs to download and install the two packages in advance, but without that, we are in a chicken-egg situation...

In fact, I am still not quite happy about the transition process, because having the new packages just around, without even loading them, while trans and prim are in the repos, already is enough to break browse...

[hulpke]

If the packages are there, the prim/trans folders are not touched any longer.

[olexandr-konovalov]

Just to summarise:

• Who: @markuspf leads SmallGroups, @hulpke TransGroups and @alex-konovalov PrimGroups.
• What's left: mostly documentation. @fingolfin's assumption is true: part of main manuals goes into packages' manuals
• How: first packages should be completed and picked up by package update mechanism. Olny AFTER that one should proceed with removing directories. Otherwise our regression tests which will be broken.
• When: this is a must for GAP 4.9 beta.
• What next: Browse and other interested packages are encouraged to adapt to the new setting.

[fingolfin]

So, what's the status of this?

• Has anybody contacted @frankluebeck and Thomas Breuer in order to get an update for the Browse package in which the conflict is resolved? (Is that even possible right now, i.e. are there "proper" APIs which Browse could use to replace its direct access to the transpkg internals?)

• What about HPC-GAP compatibility? In the GAP repository, trans, small, and prim were modified in parts to be compatible in HPC-GAP. I have not seen any efforts to retain that. So is the HPC-GAP support going to be dropped then?

• When will the new packages be "ready"? As I understand it, we want to make a GAP 4.9 release this year, but e.g. smallgrp has not been updated in the past 7 months, and primpkg in the past 5 months. So perhaps they are done already? Or if not, what has to be done? The whole process still seems rather opaque to me (and as you can perhaps see from this comment, I am rather unhappy about it).

Indeed, is there a concrete transition plan that ensures there is time between "releases" of the new packages, and the release of GAP 4.9, so that we can iron out regressions?

Ideally, I'd prefer if small, trans and prim (and their HPC-GAP counterparts) were removed from the GAP repository rather sooner than later. What is stopping us from doing that right away? OK, I guess the Browse incompatibility is one blocker -- but as I wrote above, it is unclear to me whether anybody is working on addressing that. My concern is that by the time we want to release 4.9, it still won't be adressed, and then we have to indefinitely delay GAP 4.9, leading us right back into the same bad situation that delayed GAP 4.5 for years.

On the plus side, once these dirs are split out in packages and removed from the GAP repository, I won't have to worry about them when refactoring code. Or rather, I'd still "worry" about making sure changes don't affect transpkg, primpkg, and smallpkg the same way I do for any other package in the package distribution -- but I want to either submit PRs to the GAP repositories, or the GAP repository -- but not to both.

[hulpke]

I'm not completely sure why the biurden lies on the transitive groups library and not on browse (the same issue would have come up if I had made these changes in the main repository.

Fundamentally the problem is with Browse accessing internal data directly. Soes anyone actually use browse? It would be easy to introduce dummy variable to stop syntax errors from loading the package.

We also could stop autoloading of incompatible versions of browse.

[hulpke]

If someone can tell me what the hpcgap changes do (and whether they are safe even without hpcgap) I'd be happy to port them to the transgrp package.

[fingolfin]

Well, because we normally try hard to not break existing packages without warning, even if strictly speaking the package authors are at fault.

Of course we can just do go ahead anyway, and accept that other packages get broken.

But I'd consider it quite unfriendly to not at least give a warning to the authors, so that they have a chance. I do think that's a policy we should strive to follow.

[frankluebeck]

@fingolfin wrote:

Has anybody contacted @frankluebeck and Thomas Breuer in order to get an update for the Browse package in which the conflict is resolved? (Is that even possible right now, i.e. are there "proper" APIs which Browse could use to replace its direct access to the transpkg internals?)

I accidentally stumbled over this discussion. The answer is that nobody contacted us with respect to the Browse package (which is currently tested for a new release). (And Thomas was also not contacted with respect to the CTblLib package.)

How are we supposed to notice a problem when there is none with the current master branch?

The Browse package contains a utility BrowseTransitiveGroupsInfo that is a supplement to the data access functions AllTransitiveGroups and OneTransitiveGroup. Therefore it is quite natural that it reuses some of the internal code of these functions.

Looking into the "transgrp" package it seems that it provides more data than were available in the trans directory. Therefore the mentioned function in Browse seems outdated when the transgrp package is available.

How about the following solution:

• we remove BrowseTransitiveGroupsInfo from the Browse package (this simplifies dependencies between packages),
• the transgrp package is inviited to take over the functionality from Browse (the code is in the files app/trans* of the released Browse package) and to adjust it to the new setup (of course we can help if there are questions).

[hulpke]

Fine with me. Alexander

[olexandr-konovalov]

This will be completed in #1650 which I suggested to be merged on 1st November 2017.

[olexandr-konovalov]

1650 has been merged, so this is now closed. I've created an issue at https://github.com/hulpke/transgrp/issues/26 to remind about the idea to take over BrowseTransitiveGroupsInfo from Browse to TransGrp.