Ticket #12553 (closed enhancement: fixed)

Opened 16 months ago

Last modified 6 weeks ago

Add interface for PALP polytope databases

Reported by: vbraun Owned by: was
Priority: major Milestone: sage-5.10
Component: packages: huge Keywords: PALP reflexive polytopes
Cc: novoselt, jdemeyer Work issues:
Report Upstream: N/A Reviewers: Dmitrii Pasechnik
Authors: Volker Braun Merged in: sage-5.10.beta2
Dependencies: #11763, #11634, #12544, #14467 Stopgaps:

Description (last modified by vbraun) (diff)

This ticket implements an interface to read databases of reflexive polytopes:

  • Add databases in PALP format to the polytopes_db.spkg.
  • A class PALPreader that can read the palp output and yield it in various Sage representations.
  • Finally, this ticket contains a specialized PPL lattice polytope that is fast enough to construct to go through the 500 million 4-d reflexive polytopes and compute integral points.

The updated polytopes spkg is here:  http://boxen.math.washington.edu/home/vbraun/spkg/polytopes_db-20120220.spkg

Apply:

Attachments

trac_12553_palp_database.patch Download (16.5 KB) - added by vbraun 12 months ago.
Updated patch
trac_12553_ppl_count_points.patch Download (11.3 KB) - added by vbraun 12 months ago.
Rediffed patch
trac_12553_ppl_lattice_polytope.patch Download (52.8 KB) - added by vbraun 6 weeks ago.
Updated patch

Change History

comment:1 Changed 16 months ago by vbraun

  • Status changed from new to needs_review
  • Description modified (diff)

comment:2 Changed 16 months ago by vbraun

  • Cc novoselt added
  • Description modified (diff)

comment:3 Changed 16 months ago by vbraun

  • Dependencies set to #11763, #11634

comment:4 Changed 13 months ago by vbraun

I've updated the patches with various minor improvements.

comment:5 Changed 12 months ago by Snark

I had a look and have some remarks to do:

  • now spkg-install checks for SAGE_DATA before using SAGE_DATA (the one in sage 5.0 checks SAGE_ROOT before using SAGE_DATA...), which is good!
  • the sources aren't in a directory src/ but in reflexive_polytopes/ -- isn't it against the conventions?
  • both build.sh scripts lack a shebang ;
  • is it normal that the installation is dumping everything in SAGE_DATA/reflexive_polytopes/ ? Shouldn't the spkg (1) depend on palp (2) in the build phase, use it to create the database (3) in the install phase, only install the resulting database?

comment:6 Changed 12 months ago by vbraun

I've added the shebang to the build.sh scripts, even though we are entering deep into bikeshedding territory here as these are not executed but only show how the database can be built.

I've also renamed the directory to src/, thats just something I inherited from the previous version.

All databases in Sage just copy their content. Usually they require non-trivial computer resources to build, so you specifically don't want to recompute them when you are installing Sage. In this case its actually computationally easy, but just wait until we add the 4-d case ;-)

comment:7 Changed 12 months ago by Snark

Copying the contents is ok -- what bothers me is that you copy the scripts too.

You says that "these are not executed but only show how the database can be built" -- does that mean you don't guarantee they effectively work?

comment:8 Changed 12 months ago by vbraun

Well you have my word that the scripts work :-P I think having the scripts is better than not having them. Who cares if they end up in the database directory or not.

comment:9 Changed 12 months ago by Snark

I see two reasons to care:

  1. sage is big enough ;
  2. if it's not data, it doesn't belong to SAGE_DATA.

comment:10 Changed 12 months ago by vbraun

Lets call it documentation, then, shall we?

comment:11 Changed 12 months ago by Snark

If it's documentation, it belongs into SAGE_DOC ; installing things in correct places isn't as frivolous as you seem to think...

comment:12 Changed 12 months ago by vbraun

$SAGE_DOC contains docs like the developer manual. You want me to typeset the script in sphinx and include it in the developer manual? That doesn't sound reasonable to me.

comment:13 Changed 12 months ago by Snark

Why not just cp the right files into $SAGE_DATA?

comment:14 Changed 12 months ago by vbraun

I've deleted the build.sh scripts. Updated spkg at the same location.

comment:15 Changed 12 months ago by vbraun

  • Dependencies changed from #11763, #11634 to #11763, #11634, #12544

Updating the database patch for the new PointCollection output introduced in #12544, which is one of the possible output formats.

Changed 12 months ago by vbraun

Updated patch

Changed 12 months ago by vbraun

Rediffed patch

comment:16 Changed 11 months ago by dimpase

can this ticket be rebased so that it does not depend upon anything that is in sage-pending, such as #11310 ?

comment:17 Changed 11 months ago by vbraun

  • Cc jdemeyer added

I think Jeroen just used sage-pending in #11310 since we didn't have a sage-5.3 milestone at that time and sage-5.2 is supposed to be just a new sagenb + trivial tickets. But maybe Jeroen had some other reason?

comment:18 Changed 11 months ago by jdemeyer

It's mainly because of #13109. The current plan is to merge #13109 in sage-5.2 but all follow-ups (including #11310) in sage-5.3. It's certainly not meant to be a long-term sage-pending.

Last edited 11 months ago by jdemeyer (previous) (diff)

comment:19 Changed 11 months ago by novoselt

Hi Volker, do you actually have the package for 4d polytopes which is mentioned in the patch documentation?

(By the way, maybe polytopes_reflexive_4d_db would be a better name for it?)

comment:20 Changed 11 months ago by vbraun

Sorry forgot to reply.. I haven't packaged the 4d database... 10 GB spkg? ;-) But the updated spkg contains the 2d and 3d analogues in PALP's binary database format.

comment:21 Changed 2 months ago by dimpase

somewhere down in the first patch you have

The database is very large and not installed by default. You can 
install it with the command ``install_package('polytopes_db_4d')``.

but this does not work. Was it meant to be added later?

comment:22 follow-up: ↓ 24 Changed 2 months ago by vbraun

Yes. Though there is also the question whether we want to host a 4+gb database (and I don't have a firm opinion either way)

comment:23 Changed 2 months ago by vbraun

comment:24 in reply to: ↑ 22 Changed 2 months ago by dimpase

Replying to vbraun:

Yes. Though there is also the question whether we want to host a 4+gb database (and I don't have a firm opinion either way)

IMHO, if it is hosted elsewhere, we might rather want to host an interface only. Anyhow that comment in the source about install_package('polytopes_db_4d') is misleading, and should be rectified.

comment:25 follow-up: ↓ 27 Changed 2 months ago by vbraun

  • Description modified (diff)

Updated polytopes_db-20120220.spkg to merge in the changes that others have done in the year+ that it has been lingering here.

comment:26 Changed 2 months ago by vbraun

  • Dependencies changed from #11763, #11634, #12544 to #11763, #11634, #12544, #14467

I made a ticket for the 4-d database at #14467.

comment:27 in reply to: ↑ 25 Changed 8 weeks ago by dimpase

Replying to vbraun:

Updated polytopes_db-20120220.spkg to merge in the changes that others have done in the year+ that it has been lingering here.

This spkg can only be found on http://www.stp.dias.ie/~vbraun/Sage/spkg/. Is this how this is meant to be?

comment:28 follow-up: ↓ 29 Changed 8 weeks ago by vbraun

I'm leaving Ireland and my DIAS account will be closed in the coming weeks. I uploaded the spkg to boxen, and this is where it should be. The link in the ticket description works for me.

comment:29 in reply to: ↑ 28 Changed 8 weeks ago by dimpase

  • Status changed from needs_review to positive_review

Replying to vbraun:

I'm leaving Ireland and my DIAS account will be closed in the coming weeks. I uploaded the spkg to boxen, and this is where it should be. The link in the ticket description works for me.

Wohin, Volker?

comment:30 Changed 7 weeks ago by jdemeyer

  • Milestone changed from sage-5.10 to sage-pending

Changed 6 weeks ago by vbraun

Updated patch

comment:31 Changed 6 weeks ago by jdemeyer

  • Reviewers set to Dmitrii Pasechnik

comment:32 Changed 6 weeks ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.10

comment:33 Changed 6 weeks ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-5.10.beta2
Note: See TracTickets for help on using tickets.