intersection and OldPolyhedra/Polyhedra

[mikestillman]

OldPolyhedra has a function intersection(Matrix,Matrix,Matrix,Matrix), as does Polyhedra (although it is deprecated in the newer version).

If I run tests(4, "MultiplicitySequence"), it runs fine, and this intersection method is called. However, if (in recent development branch) I change the PackageExports to include Polyhedra, not OldPolyhedra, the intersection (4 Matrices as input) appears to not be called. Instead, it appears that a more generic fold version is being called somehow, giving an error in this case. I can't tell what is happening. Any suggestions?

The reason I'm interested, is I would like to remove OldPolyhedra eventually, and there are only a few packages using it, including this one. It seems like it should be easy to remove from this package at least. I should really talk to the authors, but the problem I ran into does seem strange to me.

[mikestillman]

In reality, we probably want to remove that version of intersection, as it has an easy description in the newer Polyhedra package (I believe it is identical to polyhedronFromHData).

[d-torrance]

intersect and intersection are now synonyms (since #3742). So maybe that's the underlying problem?

[mahrud]

Ah, we might need to revert that PR, because the matrices in the method that Mike is talking about do not describe the same type of object, they are equalities and inequalities! See here.

Edit: but I think we can reapply that PR after completely deprecating that form of intersection in OldPolyhedra, or better yet deprecating the package altogether.

[d-torrance]

Why revert the intersection change? Everything works fine in its current state.

If we were to update MultiplicitySequence to use Polyhedra instead of OldPolyhedra, then we can swap out the 4-matrix intersection calls with polyhedronFromHData.

[mahrud]

OldPolyhedra does not work with the new intersection change, unless you rename intersection there.

[d-torrance]

It does on my system... intersection is overwritten by the OldPolyhedra version once I load the package.

i1 : intersection === intersect

o1 = true

i2 : needsPackage "OldPolyhedra";
 -- ... lots of "shadowed by a symbol" warnings ...

i3 : intersection === intersect

o3 = false

i4 : M = matrix {{1, 2, 3}, {3, 2, 1}, {3, 1, 2}, {-1, -1, -1}};

              4       3
o4 : Matrix ZZ  <-- ZZ

i5 : v = matrix {{1}, {2}, {3}, {4}};

              4       1
o5 : Matrix ZZ  <-- ZZ

i6 : N = matrix {{1, 2, 0}};

              1       3
o6 : Matrix ZZ  <-- ZZ

i7 : w = matrix {{2}};

              1       1
o7 : Matrix ZZ  <-- ZZ

i8 : intersection(M, v, N, w)

o8 = {ambient dimension => 3           }
      dimension of lineality space => 0
      dimension of polyhedron => 2
      number of facets => 3
      number of rays => 0
      number of vertices => 3

o8 : Polyhedron

[mahrud]

Okay, so we just need to make sure it is the final dependency of a package. #3774 does this for MultiplicitySequence in ce07730

[mahrud]

It looks like only 3 packages use it:

./MultiplicitySequence.m2:26:        "OldPolyhedra",
./SRdeformations.m2:13:     	PackageImports => { "ConvexInterface", "OldPolyhedra" },
./OldToricVectorBundles.m2:38:    PackageExports => {"Isomorphism", "OldPolyhedra"}

I've asked around what are the advantages of OldPolyhedra over Polyhedra that prevents people from using Polyhedra and I haven't got anything concrete that needs to be improved yet.

[mikestillman]

@mahrud I think the advantage of OldPolyhedra is simply that the authors aren't around or have changed interest, and so they haven't been changed. For the first two, I think the changes look fairly easy. How about OldToricVectorBundles, how does that compare to the new toric vector bundles package? @ggsmith ?

[mahrud]

ToricVectorBundles has had some updates by recently. @nilten are there strong reasons in favor of keeping OldToricVectorBundles around?

[nilten]

Not really. When the Polyhedra package got updated (and the original one became "OldPolyhedra") the original ToricVectorBundles package didn't work with the new Polyhedra. @lkastner fixed all the bits that didn't work; the changes were significant enough that he renamed the old package "OldToricVectorBundles" (since it works with "OldPolyhedra") and the fixed one became "ToricVectorBundles". The functionality should be almost identical.

[mahrud]

Thanks! @mikestillman @d-torrance in this case, I think it's probably safe to move OldPolyhedra and OldToricVectorBundles to the undistributed-packages directory in this release.

[d-torrance]

I played around with this for a bit. MultiplicitySequence is easy to port to Polyhedra, but SRdeformations is not since it defines a few method functions with the same names as Polyhedra ones (facets, simplex, and minimalNonFaces) with different options and typical values.