Opened 4 years ago

Closed 4 years ago

#18779 closed enhancement (fixed)

polytopes.gosset_3_21 and graphs.GossetGraph

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.8
Component: geometry Keywords:
Cc: vdelecroix, dimpase Merged in:
Authors: Nathann Cohen Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 6527f4b (Commits) Commit: 6527f4b41f580d470c734f30c109bc25049bfb29
Dependencies: Stopgaps:

Description

https://en.wikipedia.org/wiki/Gosset_graph https://en.wikipedia.org/wiki/3_21_polytope

The graph is not generated from the polytope, as it takes ~16 seconds.

Nathann

Change History (15)

comment:1 Changed 4 years ago by ncohen

  • Branch set to public/18779
  • Status changed from new to needs_review

comment:2 Changed 4 years ago by git

  • Commit set to c3195be870d62e0815b0fb5ba1502a87a9bb26b4

Branch pushed to git repo; I updated commit sha1. New commits:

c3195betrac #18779: polytopes.gosset_3_21 and graphs.GossetGraph

comment:3 follow-up: Changed 4 years ago by chapoton

  • add gosset to the list of polytopes at the beginning of file
  • use the correct f-vector, even if not tested

comment:4 Changed 4 years ago by git

  • Commit changed from c3195be870d62e0815b0fb5ba1502a87a9bb26b4 to 2c3e33c797afda4b1cb6d222be542e3b91e0bc09

Branch pushed to git repo; I updated commit sha1. New commits:

2c3e33ctrac #18779: Reviewer's (dead right) comments

comment:5 in reply to: ↑ 3 Changed 4 years ago by ncohen

  • add gosset to the list of polytopes at the beginning of file
  • use the correct f-vector, even if not tested

Both done. Sorry.

Nathann

comment:6 Changed 4 years ago by git

  • Commit changed from 2c3e33c797afda4b1cb6d222be542e3b91e0bc09 to 24ea250eeb70dfeacf881ae8622e7f42a908a38c

Branch pushed to git repo; I updated commit sha1. New commits:

bfdd09dMerge branch 'public/gap478' of git://trac.sagemath.org/sage into develop
9a9458aMerge branch 'public/18626' of git://trac.sagemath.org/sage into develop
4211dc8Merge branch 'u/dimpase/18827' of git://trac.sagemath.org/sage into develop
4ab08fcMerge branch 'u/dimpase/18830' of git://trac.sagemath.org/sage into develop
43f713cMerge branch 'develop' of git://trac.sagemath.org/sage into develop
1023880Merge remote-tracking branch 'trac/public/18779' into gosset
24ea250capitalise Gosset

comment:7 follow-up: Changed 4 years ago by dimpase

oops, as usual, I only meant to push the last commit. After this change, I'd be happy to set it to positive review.

comment:8 Changed 4 years ago by git

  • Commit changed from 24ea250eeb70dfeacf881ae8622e7f42a908a38c to 1b391ddfcd53789652805eb8fcb2929338567c04

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4a4ee0ctrac #18779: polytopes.gosset_3_21 and graphs.GossetGraph
b498a9etrac #18779: Reviewer's (dead right) comments
1b391ddcapitalise Gosset

comment:9 Changed 4 years ago by git

  • Commit changed from 1b391ddfcd53789652805eb8fcb2929338567c04 to 6527f4b41f580d470c734f30c109bc25049bfb29

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6527f4bcapitalise Gosset

comment:10 in reply to: ↑ 7 Changed 4 years ago by ncohen

oops, as usual, I only meant to push the last commit.

Fixed.

After this change, I'd be happy to set it to positive review.

I also modified the graph doctest which calls the polytope. Good to go?

Nathann

comment:11 follow-up: Changed 4 years ago by dimpase

  • Status changed from needs_review to positive_review

Looks good.

These 16 sec to build the graph from polytope highlight the need to have a method to do so directly, without constructing all faces. In fact, it can be done by just calling an LP solver the number of times equal to the number of pairs of vertices...

comment:12 in reply to: ↑ 11 Changed 4 years ago by ncohen

These 16 sec to build the graph from polytope highlight the need to have a method to do so directly, without constructing all faces.

I did that in #18860, but... Well, the function does not work :-P

Actually, it works to build this polyhedron, but I hard-coded a constant '26'. If you can figure out what should appear instead of 26, then we will proably have a huge speedup for this .graph function :-P

Nathann

comment:13 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name...

comment:14 Changed 4 years ago by ncohen

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_work to positive_review

comment:15 Changed 4 years ago by vbraun

  • Branch changed from public/18779 to 6527f4b41f580d470c734f30c109bc25049bfb29
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.