Opened 12 years ago

Closed 12 years ago

#7820 closed enhancement (fixed)

upgrade gfan to latest release (0.4plus)

Reported by: AlexGhitza Owned by: tbd
Priority: major Milestone: sage-4.3.2
Component: packages: standard Keywords:
Cc: mhampton, was Merged in: sage-4.3.2.alpha0
Authors: Alex Ghitza, David Kirkby Reviewers: Marshall Hampton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by AlexGhitza)

The new spkg is at

http://sage.math.washington.edu/home/ghitza/gfan-0.4plus.spkg

See http://www.math.tu-berlin.de/~jensen/software/gfan/gfan.html

Release 0.4plus has improved performance and a lot of new functionality.

This also takes care of the following issues: remove the debian dist directory (see #5903), clarify the license (see #3043), and separate the clean upstream from the patches needed to build in Sage (see #3338).

Attachments (1)

trac_7820.patch (4.3 KB) - added by AlexGhitza 12 years ago.
apply after the gfan-0.4plus spkg

Download all attachments as: .zip

Change History (22)

comment:1 Changed 12 years ago by AlexGhitza

  • Description modified (diff)

comment:2 Changed 12 years ago by AlexGhitza

  • Authors set to Alex Ghitza
  • Status changed from new to needs_work

An updated spkg is up at

http://sage.math.washington.edu/home/ghitza/gfan-0.4plus.spkg

Some fixes are needed in the Sage library because of the upgrade. I am attaching a patch that deals with the most obvious one, but there are some doctest failures left:

sage -t -long "devel/sage-main/sage/rings/polynomial/groebner_fan.py"
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 301:
    sage: pf.rays()
Expected:
    [[1, 0, 0], [-2, -1, 0], [1, 1, 0], [0, -1, 0], [-1, 1, 0]]
Got:
    [[-1, 0, 1], [-1, 1, 0], [1, -2, 1], [1, 1, -2], [2, -1, -1]]
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 354:
    sage: pf._str_()
Expected:
    '_application PolyhedralFan\n_version 2.2\n_type PolyhedralFan\n\nAMBIENT_DIM\n3\n\nDIM\n3\n\nLINEALITY_DIM\n0\n\nRAYS\n1 0 0\t# 0\n0 1 0\t# 1\n0 0 1\t# 2\n\nN_RAYS\n3\n\nLINEALITY_SPACE\n\nORTH_LINEALITY_SPACE\n0 0 1\n0 1 0\n1 0 0\n\nF_VECTOR\n1 3 3 1\n\nCONES\n{}\t# Dimension 0\n{0}\t# Dimension 1\n{1}\n{2}\n{0 1}\t# Dimension 2\n{0 2}\n{1 2}\n{0 1 2}\t# Dimension 3\n\nMAXIMAL_CONES\n{0 1 2}\t# Dimension 3\n\nPURE\n1\n'
Got:
    '_application PolyhedralFan\n_version 2.2\n_type PolyhedralFan\n\nAMBIENT_DIM\n3\n\nDIM\n3\n\nLINEALITY_DIM\n0\n\nRAYS\n0 0 1\t# 0\n0 1 0\t# 1\n1 0 0\t# 2\n\nN_RAYS\n3\n\nLINEALITY_SPACE\n\nORTH_LINEALITY_SPACE\n1 0 0\n0 1 0\n0 0 1\n\nF_VECTOR\n1 3 3 1\n\nMY_EULER\n0\n\nSIMPLICIAL\n1\n\nPURE\n1\n\nCONES\n{}\t# Dimension 0\n{0}\t# Dimension 1\n{1}\n{2}\n{0 1}\t# Dimension 2\n{0 2}\n{1 2}\n{0 1 2}\t# Dimension 3\n\nMAXIMAL_CONES\n{0 1 2}\t# Dimension 3\n'
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 413:
    sage: pf.rays()
Expected:
    [[1, 0, 0], [-2, -1, 0], [1, 1, 0], [0, -1, 0], [-1, 1, 0]]
Got:
    [[-1, 0, 1], [-1, 1, 0], [1, -2, 1], [1, 1, -2], [2, -1, -1]]
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 138:
    sage: _cone_parse(tstr.fan_dict['CONES'])
Expected:
    {1: [[0], [1], [3], [2], [4]], 2: [[2, 4]]}
Got:
    {1: [[0], [1], [2], [3], [4]], 2: [[2, 3]]}
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 824:
    sage: pf.rays()
Expected:
    [[1, 0, 0], [0, 1, 0], [0, 0, 1]]
Got:
    [[0, 0, 1], [0, 1, 0], [1, 0, 0]]
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 1243:
    sage: G.tropical_basis()
Exception raised:
    Traceback (most recent call last):
      File "/virtual/scratch/ghitza/sage-4.3/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/virtual/scratch/ghitza/sage-4.3/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/virtual/scratch/ghitza/sage-4.3/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_48[5]>", line 1, in <module>
        G.tropical_basis()###line 1243:
    sage: G.tropical_basis()
      File "/virtual/scratch/ghitza/sage-4.3/local/lib/python/site-packages/sage/rings/polynomial/groebner_fan.py", line 1267, in tropical_basis
        X = [S(f) for f in B]
      File "multi_polynomial_libsingular.pyx", line 608, in sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular.__call__ (sage/rings/polynomial/multi_polynomial_libsingular.cpp:5471)
        raise TypeError
    TypeError
**********************************************************************
File "/virtual/scratch/ghitza/sage-4.3/devel/sage-main/sage/rings/polynomial/groebner_fan.py", line 1294:
    sage: pf.rays()
Expected:
    [[-1, 0, 0]]
Got:
    [[-2, 1, 1]]
**********************************************************************
7 items had failures:
   1 of   7 in __main__.example_11
   1 of   6 in __main__.example_13
   1 of   7 in __main__.example_17
   1 of   8 in __main__.example_3
   1 of   6 in __main__.example_35
   1 of   6 in __main__.example_48
   1 of   7 in __main__.example_50
***Test Failed*** 7 failures.
For whitespace errors, see the file /home/ghitza/.sage//tmp/.doctest_groebner_fan.py
	 [8.1 s]
exit code: 1024
 
----------------------------------------------------------------------
The following tests failed:


	sage -t -long "devel/sage-main/sage/rings/polynomial/groebner_fan.py"
Total time for all tests: 8.2 seconds

I can try to figure out what's happening, but Marshall is likely to be better at it.

comment:3 Changed 12 years ago by mhampton

Yes, I can try to work on fixing those issues. This relates a little bit to some major refactoring of the polyhedron class over at ticket #7109. If you could review that I'd greatly appreciate it.

Thanks very much for working on the gfan upgrade.

comment:4 Changed 12 years ago by mhampton

I'm starting to think that rays doctest might have exposed a bug in gfan itself, I'll check with Anders.

comment:5 Changed 12 years ago by mhampton

The new output is correct; the rays for that example are not uniquely determined. So the doctests just need to be changed to match the new output.

comment:6 follow-up: Changed 12 years ago by drkirkby

It would be good if the Makefile could be changed so that the C compiler is set by CC and the C++ compiler by CXX, and the flags CFLAGS and CXXFLAGS used.

The Sun compiler was not happy with the earlier version.

I dont mind trying to sort the makefile out if you want.

Dave

comment:7 in reply to: ↑ 6 Changed 12 years ago by AlexGhitza

Replying to drkirkby:

It would be good if the Makefile could be changed so that the C compiler is set by CC and the C++ compiler by CXX, and the flags CFLAGS and CXXFLAGS used.

The Sun compiler was not happy with the earlier version.

I dont mind trying to sort the makefile out if you want.

Sure! That would make it easier for me.

comment:8 Changed 12 years ago by drkirkby

I'll do that today, by 1800 GMT today (7 hours from now).

It was coincidental, but I'd just hit the problem on version 0.3 and posted something to sage-devel about this. William was willing to remove gfan, as he was no aware of anyone using it. You might want to put a comment on sage-devel about it.

Dave

comment:9 Changed 12 years ago by drkirkby

I'm a bit concerned about the number of compiler warnings here from g++, even though -Wall was not added.

On the 0.3 release, the Sun compiler rejected some code, saying things were masking others, and functions expected to return something did not.

I'll get the Makefile fixed so gfan 0.4plus it at least attempts a build with Sun's compiler, but I suspect Sun's will reject some of the code as being invalid.

Dave

comment:10 follow-up: Changed 12 years ago by drkirkby

The 'src' directory at

http://sage.math.washington.edu/home/ghitza/gfan-0.4plus.spkg

has no 'makefile' yet when I download the source code from

http://www.math.tu-berlin.de/~jensen/software/gfan/gfan0.4plus.tar.gz

there is a makefile.

I think we should keep that, then overwrite it with a patch, so there is a record of the actual source code. i.e. something like

cp patches/makefile src/makefile

That way, the original source is untouched. I rather start with the original code, and make changes to that, rather than someone elses makefile.

Dave

comment:11 Changed 12 years ago by drkirkby

  • Status changed from needs_work to needs_review

Well, I'm 3 minutes late, but I hope you will excuse that.

I've put a new spkg at

http://boxen.math.washington.edu/home/kirkby/portability/gfan-0.4plus/

I used a clean source, and just edited spkg-install and made patches/Makefile.

Can you check I've note undone anything you have done, and check it works on a couple of systems. I've only checked on Solaris here.

Dave

comment:12 Changed 12 years ago by drkirkby

  • Authors changed from Alex Ghitza to Alex Ghitza, David Kirkby

comment:13 in reply to: ↑ 10 Changed 12 years ago by AlexGhitza

Replying to drkirkby:

The 'src' directory at

http://sage.math.washington.edu/home/ghitza/gfan-0.4plus.spkg

has no 'makefile' yet when I download the source code from

http://www.math.tu-berlin.de/~jensen/software/gfan/gfan0.4plus.tar.gz

there is a makefile.

I think we should keep that, then overwrite it with a patch, so there is a record of the actual source code. i.e. something like

cp patches/makefile src/makefile

That way, the original source is untouched. I rather start with the original code, and make changes to that, rather than someone elses makefile.

Dave

I'm not sure what spkg file you're looking at. I just rechecked the one I gave above and it has the original Makefile in src/ and a modified one in patches/.

Anyway, it's not important. I'm looking at your spkg now.

comment:14 Changed 12 years ago by drkirkby

Sorry about the confusion.

I can only assume I confused it with the 0.3 version by mistake. I certainly downloaded your one.

I downloaded the gfan-0.4plus source code, and put that in the package.

comment:15 Changed 12 years ago by AlexGhitza

It looks good, thanks for this!

So far I've tested it on 32-bit archlinux and 64-bit archlinux without problems. Same on 32-bit MacOSX 10.5.

There are a couple of minor things in your spkg, which I can easily fix today: we normally strip the original sources of documentation and similar things. In this case, we get rid of src/doc, src/examples, and src/homepage. This gives an spkg that's 220kb instead of 680kb. There are also a few typos in Makefile and SPKG.txt.

As I said, this is great, and I can make these minor fixes today.

comment:16 Changed 12 years ago by drkirkby

It's good to hear it was mostly there.

At least one can to build gfan with Sun's compiler now, even though it soon fails. Before it was impossible to get anything useful done, as gcc was hard-coded in the Makefile. Now at least one can see the error messages. Unfortunately, whilst I know C, I do not know C++, so don't have much clue about how to resolve the issues. I'll forward them upstream.

Dave

Changed 12 years ago by AlexGhitza

apply after the gfan-0.4plus spkg

comment:17 follow-up: Changed 12 years ago by AlexGhitza

  • Description modified (diff)

OK, I have replaced the spkg with one that has the small fixes to David's version. See the ticket description for the URL. This spkg also patches src/polynomial.cpp to fix a bug that appeared in gfan-0.4plus and was caught by one of our doctests (YAY for doctests!)

I have also replaced the patch to the Sage library with one that deals with all the issues raised by the upgrade. All of this is now ready for review.

comment:18 in reply to: ↑ 17 Changed 12 years ago by AlexGhitza

Replying to AlexGhitza:

OK, I have replaced the spkg with one that has the small fixes to David's version. See the ticket description for the URL. This spkg also patches src/polynomial.cpp to fix a bug that appeared in gfan-0.4plus and was caught by one of our doctests (YAY for doctests!)

Forgot to mention that the bug was reported upstream, and Anders Jensen quickly sent us the fix that we are now using. It will be incorporated in the next version of gfan.

comment:19 Changed 12 years ago by AlexGhitza

  • Description modified (diff)

comment:20 Changed 12 years ago by mhampton

  • Status changed from needs_review to positive_review

I've tested this and I think it looks good. There are some minor conflicts with 7109 but I am willing to rebase 7109 against this if it is merged first.

I might not have time until this summer to extend the gfan interface but I am interested in doing so. If anyone beats me to it I am happy to review.

comment:21 Changed 12 years ago by mvngu

  • Merged in set to sage-4.3.2.alpha0
  • Resolution set to fixed
  • Reviewers set to Marshall Hampton
  • Status changed from positive_review to closed

Merged in this order:

  1. merged gfan-0.4plus.spkg in the standard spkg repository
  2. trac_7820.patch
Note: See TracTickets for help on using tickets.