Refactoring Polyhedra

[lkastner]

This issue is dedicated to discussing various questions that came up while refactoring Polyhedra.m2. The current status can be found at my fork and can hopefully be merged soon. The bugs 1, in minkowskiSum and at various other places have been fixed. Many tests have been and will be added. The following should be discussed:

1. What should the output for a polytope look like? Should it display any of the properties or just the type? Since everything was switched to lazy evaluation, we certainly would not want to compute anything here.
2. What about other packages connected to this refactor, see the following pull request. Does PolyhedralObjects.m2 make sense without Polyhedra.m2? Is Polyhedra2.m2 still being worked on (this duplicates old code, that has been refactored)? @JosephineYu
3. As I understand I should not just fix packages that rely on Polyhedra.m2 to adjust them to the new version. What is best practice here?

Additionally I have the following minor questions:

1. There is now an interface to lrs. What is best practice for the tests for this interface? Can I make tests that only run if lrs is installed? This concerns future interfaces as well.
2. Can I make tests that should fail, i.e. check that an error is thrown?
3. Are there any bugs that I am not aware of? Should I write to the google group asking for bugs in Polyhedra?

Can I send a pull request, even though the documentation is not fully updated yet? All the old and new tests work, as well as all the examples from the documentation. Except for a few exceptions, all functions still exhibit the same behaviour.

[JosephineYu]

I haven't been working on Polyhedra2.m2. The original ideas was that gfanInterface2 would depend on PolyhedralObjects.m2 but not on Polyhedra.m2. I haven't been keeping up with the current status of gfanInterface2,Tropical, etc. You might want to check with Diane.

[DanGrayson]

Re: "What is best practice for the tests for this interface? Can I make tests that only run if lrs is installed? "

For interfaces to proprietary software, such as Maple, we don't run tests. There are two ways to do that: have no tests; or use the option CacheExampleOutput => true. The latter will result in no tests being run automatically, and you must run the tests manually and add the output to the repository yourself whenever you think of it.

There seems to be an ubuntu package for it: http://packages.ubuntu.com/precise/i386/lrslib/filelist . If the software is free, we could add it as a library, as we do with many other packages, so, in case it's not installed on the system, we download the source and build it and ship it ourselves. That might be best, if you want people to be able to use it where it's not easily available. You could try adding the libary yourself as a subdirectory of the directory libraries.

[DanGrayson]

Re: "Can I make tests that should fail, i.e. check that an error is thrown?"

Yes, see "try ... catch ... ". But there is no way to tell whether the correct error was thrown! The function generateAssertions even knows that trick:

i10 : generateAssertions " 1/0 "

o10 = 
      assert( (try  1/0  else oops) === oops );

[DanGrayson]

Re: "Can I send a pull request, even though the documentation is not fully updated yet?"

The major requirement for a pull request is that it be error free.

[DanGrayson]

Here's the home page of lrs: http://cgm.cs.mcgill.ca/~avis/C/lrs.html

[lkastner]

Thank you for your answers @DanGrayson .

Some minor issues are: @mikestillman @kathlenkohn @dmaclagan @nilten

1. The name 'intersection' is used mutiple times, for the intersection giving a cone, a polyhedron and the intersection of cones or polyhedra. The first two should be constructors and have better names, something like 'coneFromInequalities', except that this is too long. Thoughts?
2. Can the 'saveSession' method go away? One could always save the buffer if one wants the output. Also there is no 'loadSession' functionality, rendering this half-useless.
3. I commented the displaying code. Now whenever a polyhedral object is output, M2 will just stick to the default and print 'Cone', 'Polytope', etc. I had a look at 'NormalToricVarieties.m2', which seems to do the same. Is that acceptable?
4. Wishes? Bugs? Problems?

[lkastner]

Here is another problem: 'volume' projects the polytope down if dimension and ambient dimension do not agree. Is this behaviour we want?

[lkastner]

@ggsmith

[lkastner]

I will now send the first pull request. There are three commits on my master branch concerning other packages:

1. 9797233b765b816a957cc535a1189c70702e369e this commit fixes the usage of Polyhedra.m2 functions in MonomialAlgebras.m2.
2. 657bf05aa4861075f45d778814ee52cd087d51b9 and 6df0049e6aa798c3debda2a778361d1f8e5568f3 were my attempts to fix the usage of Polyhedra.m2 functions in LatticePolytopes.m2. The second commit message is incorrect, sorry. There seems to be some issue with 'latticePoints' from NormalToricVarieties.m2.

I thought these might help you what the issues might be. You can just cherry-pick around them and send the diff of these commits to the authors as a proposal on what to change.

Please tell me if I broke something else or should do something differently.

[DanGrayson]

I'd appreciate it if, instead, you would just flag all the authors with @names in your pull request and ask them to comment.