Opened 9 years ago
Closed 7 years ago
#11395 closed enhancement (fixed)
Upgrade Gfan to latest release (version 0.5)
Reported by: | mhampton | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-5.5 |
Component: | packages: standard | Keywords: | tropical geometry |
Cc: | Merged in: | sage-5.5.beta1 | |
Authors: | Marshall Hampton, François Bissey | Reviewers: | François Bissey, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
From the Gfan page: "Gfan version 0.5 (January 23th 2011, major update, Gröbner bases/fans over the integers (experimental), p-adic Gröbner bases/complexes (experimental), fan homology computations, exporting of fans in Polymake's new xml format, performance improvements (gfan is now faster than TOPCOM for secondary fan computations) and much more.) Updated January 25th 2011 (xml format and installation instructions). "
We should upgrade to this release.
New spkg available at http://spkg-upload.googlecode.com/files/gfan-0.5.p0.spkg
Apply:
Attachments (3)
Change History (35)
comment:1 Changed 9 years ago by
comment:2 Changed 9 years ago by
A candidate package is at http://sage.math.washington.edu/home/mhampton/gfan-0.5.spkg
I will take your suggestion on patching the interface. I am aware of some other issues that need to be fixed; I was going to put those on another ticket but if 0.5 changes its output anyway I suppose it makes sense to put both the update and interface patches in one ticket.
comment:3 Changed 9 years ago by
I had kind of started work on that. I am starting to hate when sage-on-gentoo is too much ahead and it breaks tests in non trivial ways. Most of the tests mentioned by Christopher are broken by trivial changes of formatting or ordering. But that one I think needs careful checking:
********************************************************************** File "/usr/share/sage/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 606: sage: g.weight_vectors() Expected: [(3, 7, 1), (5, 1, 2), (7, 1, 4), (1, 1, 4), (1, 1, 1), (1, 4, 1), (1, 4, 10)] Got: [(3, 7, 1), (5, 1, 2), (7, 1, 4), (5, 1, 4), (1, 1, 1), (1, 4, 8), (1, 4, 10)] **********************************************************************
It is not obvious that the two answers are equivalent, I was going to submit it to the opinion of sage-devel on Monday.
On another note, do you know what this section of the Makefile is all about
ifeq ($(sagepath),) SAGE_LINKOPTIONS = SAGE_INCLUDEOPTIONS = else SAGE_LINKOPTIONS = -L$(sagepath)/ -lpython2.6 -lcsage -lsingular SAGE_INCLUDEOPTIONS = -I $(sagepath)/ SAGE_OBJECTS = sage.o sage_link.so sage_link.so: sage_link.pyx setup.py python setup.py build_ext --inplace --pyrex-include-dirs=$(SAGE_ROOT)/devel/sage/ endif
It is not build by default and the cython source is missing (the resulting .cpp is present) but it looks like someone is looking at producing their own sage-extension.
comment:4 Changed 9 years ago by
I just had a quick look at your spkg. You didn't use "hg rm" to remove the files that are not needed anymore in patches. So hg status says this
francois@vrooom ~/Downloads/gfan-0.5 $ hg status ! patches/gcc45_linalg.cpp.patch ! patches/gcc45_matrix.h.patch ! patches/linalg.cpp ! patches/matrix.h ! patches/polynomial.cpp ! patches/polynomial.cpp.diff
comment:5 Changed 9 years ago by
OK, I removed the unneeded patches.
I don't think we want to change 'gfan' to 'gfan_bases' in the interface, that should be done in rings/polynomial/groebner_fan.py. I'll try to do that this week, along with other interface updates.
comment:6 Changed 9 years ago by
OK, looks better now. SPKG.txt could be more verbose about your changes (you dropped patches because they are not needed anymore and so on). However the spkg looks in good shape now. I'll give you a review (hopefully positive) once you post the patches.
comment:7 Changed 8 years ago by
- Status changed from new to needs_review
comment:8 Changed 8 years ago by
- Status changed from needs_review to needs_work
We need some patch to use gfan 5.0 without at least generating noise. Putting in needs_work.
comment:9 Changed 8 years ago by
comment:10 Changed 8 years ago by
Is the patch safe to use with earlier version of gcc? c++, any standard, is a always a major source of headaches too me.
comment:11 Changed 7 years ago by
I hope to make some progress on this ticket for Sage-Combinat Days 40, July 9-13. So far, I have a working spkg on my mac, but I need to figure out if the gcc-4.5.0 patches need to be rebased for portability. I also want to add in some interface improvements (supporting symmetry for example).
In case anyone is interested in helping I will post my progress. Working spkg: http://sage.math.washington.edu/home/mhampton/gfan-0.5.p0.spkg
Current patch: http://sage.math.washington.edu/home/mhampton/trac_11395_update_gfan_to_0.5.patch
comment:12 Changed 7 years ago by
It is a pleasure to see some progress on this. I will have a look.
comment:13 Changed 7 years ago by
Patch works great but there are more instances of tests of gfan in the documentation:
sage -t -long -force_lib "devel/sage/doc/fr/tutorial/tour_advanced.rst" ********************************************************************** File "/usr/share/sage/devel/sage/doc/fr/tutorial/tour_advanced.rst", line 68: sage: F.reduced_groebner_bases () Expected: [[-c^2 + b*d, -b*c + a*d, -b^2 + a*c], [c^2 - b*d, -b*c + a*d, -b^2 + a*c], [c^2 - b*d, b*c - a*d, -b^2 + a*c, -b^3 + a^2*d], [c^2 - b*d, b*c - a*d, b^3 - a^2*d, -b^2 + a*c], [c^2 - b*d, b*c - a*d, b^2 - a*c], [-c^2 + b*d, b^2 - a*c, -b*c + a*d], [-c^2 + b*d, b*c - a*d, b^2 - a*c, -c^3 + a*d^2], [c^3 - a*d^2, -c^2 + b*d, b*c - a*d, b^2 - a*c]] Got: [[-c^2 + b*d, -b*c + a*d, -b^2 + a*c], [-c^2 + b*d, b^2 - a*c, -b*c + a*d], [-c^2 + b*d, b*c - a*d, b^2 - a*c, -c^3 + a*d^2], [c^3 - a*d^2, -c^2 + b*d, b*c - a*d, b^2 - a*c], [c^2 - b*d, -b*c + a*d, -b^2 + a*c], [c^2 - b*d, b*c - a*d, -b^2 + a*c, -b^3 + a^2*d], [c^2 - b*d, b*c - a*d, b^3 - a^2*d, -b^2 + a*c], [c^2 - b*d, b*c - a*d, b^2 - a*c]]
That's the French one but we have the same in English, Russian and German (de) version.
comment:14 Changed 7 years ago by
Attaching a patch for the tutorial. Do you want me to clean up your spkg a little bit? We should only keep the patch for gcc 4.7 and really switch to patch. Of course SPKG.txt needs updating and cleaning after your work. There doesn't seem to be any tests so spkg-check. I can do some of that clean up if you want.
comment:15 Changed 7 years ago by
If you can do some of that soon (next two or three days), then go for it, that would be great. In the meantime I will focus on adding support for symmetry and maybe some other new things in gfan-0.5 like computing over ZZ.
comment:16 Changed 7 years ago by
OK I am doing this now. A bit more work than I had expected but doable.
comment:17 Changed 7 years ago by
The file app_minkowski.cpp had changes in it that are not in the patch file you shipped. I cannot see the equivalent modification in Gentoo for gcc-4.7.0 so I am skipping it.
comment:18 Changed 7 years ago by
- Description modified (diff)
- Reviewers set to François Bissey
- Status changed from needs_work to needs_review
OK put a new spkg up. I have gone through the whole TODO list so the changes have been a bit invasive. I made a new Makefile that uses CXXFLAGS, LDFLAGS and CPPFLAGS - it could possibly use a little bit more love before sending upstream. Scrap that it would need a fair rewrite to get rid of a lot of stuff. Patch is now used throughout. Removal of the old gfan install is now done after the build and done correctly - which it wasn't before because of a misplaced double-quote.
I think that sums it up. Over to you Marshall to see if you like the whole thing.
comment:19 Changed 7 years ago by
- Description modified (diff)
comment:20 Changed 7 years ago by
- Description modified (diff)
comment:21 Changed 7 years ago by
I credited you (François Bissey) in the SPKG.txt, and I have uploaded a patch for the interface that I think greatly improves the functionality as well as supporting the new version. I am tempted to do more refactoring so I will not flag this for review quite yet.
comment:22 Changed 7 years ago by
I should mention that I am not really qualified to review the changes made to the spkg-install, but I will test this on a few more linux and OS X systems at least.
comment:23 Changed 7 years ago by
Actually I may make one more change. I defined PREFIX on the line calling make but the variable is never used. It would be if we were using the makefile to perform the installation but we don't. I tested the spkg on linux on power7. And as a final note I should add that gfan itself doesn't come with either tests or documentation. A curiosity is that gfan seem to be mostly c++ but link against libgmp rather than libgmpxx.
comment:24 Changed 7 years ago by
- Description modified (diff)
OK I updated the spkg. In the end I kept the gcc-4.7 patch for app_minkowsky.cpp as before to minimize the burden of review.
As mentioned I removed the setting of PREFIX and made a comment about it.
comment:25 Changed 7 years ago by
I get a doctest error on sage-5.4.beta1 due to #12544
sage -t devel/sage-main/sage/rings/polynomial/groebner_fan.py ********************************************************************** File "/home/vbraun/opt/sage-5.4.beta1/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 504: sage: [q.facet_normals() for q in fan] Expected: [(M(0, -1, 0), M(-1, 0, 0)), (M(0, 0, -1), M(-1, 0, 0)), (M(0, 0, 1), M(-1, 0, 0)), (M(0, 1, 0), M(-1, 0, 0)), (M(0, 0, -1), M(0, -1, 0)), (M(0, 0, 1), M(0, -1, 0)), (M(0, 1, 0), M(0, 0, -1)), (M(0, 1, 0), M(0, 0, 1)), (M(1, 0, 0), M(0, -1, 0)), (M(1, 0, 0), M(0, 0, -1)), (M(1, 0, 0), M(0, 0, 1)), (M(1, 0, 0), M(0, 1, 0))] Got: [M( 0, -1, 0), M(-1, 0, 0) in 3-d lattice M, M( 0, 0, -1), M(-1, 0, 0) in 3-d lattice M, M( 0, 0, 1), M(-1, 0, 0) in 3-d lattice M, M( 0, 1, 0), M(-1, 0, 0) in 3-d lattice M, M(0, 0, -1), M(0, -1, 0) in 3-d lattice M, M(0, 0, 1), M(0, -1, 0) in 3-d lattice M, M(0, 1, 0), M(0, 0, -1) in 3-d lattice M, M(0, 1, 0), M(0, 0, 1) in 3-d lattice M, M(1, 0, 0), M(0, -1, 0) in 3-d lattice M, M(1, 0, 0), M(0, 0, -1) in 3-d lattice M, M(1, 0, 0), M(0, 0, 1) in 3-d lattice M, M(1, 0, 0), M(0, 1, 0) in 3-d lattice M]
comment:26 Changed 7 years ago by
- Description modified (diff)
comment:27 Changed 7 years ago by
- Status changed from needs_review to needs_work
needs_work because of Volker's failure.
comment:28 Changed 7 years ago by
- Description modified (diff)
- Milestone changed from sage-5.4 to sage-5.5
- Reviewers changed from François Bissey to François Bissey, Volker Braun
- Status changed from needs_work to positive_review
Added a trivial fix, setting it back to positive review.
comment:29 Changed 7 years ago by
- Status changed from positive_review to needs_work
The first patch needs a proper commit message.
comment:30 Changed 7 years ago by
- Description modified (diff)
- Status changed from needs_work to positive_review
I've added the missing commit message.
comment:31 Changed 7 years ago by
Thanks I had completely forgotten about this.
comment:32 Changed 7 years ago by
- Merged in set to sage-5.5.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
On Gentoo we already did this, so here a little summary what to do:
cmd = 'gfan'
tocmd = 'gfan_bases'
because the earlier is deprecated. If we do not do this, gfan prints: