Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade gfan to latest release (0.4plus) #7820

Closed
aghitza opened this issue Jan 2, 2010 · 26 comments
Closed

upgrade gfan to latest release (0.4plus) #7820

aghitza opened this issue Jan 2, 2010 · 26 comments

Comments

@aghitza
Copy link

aghitza commented Jan 2, 2010

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).

CC: @sagetrac-mhampton @williamstein

Component: packages: standard

Author: Alex Ghitza, David Kirkby

Reviewer: Marshall Hampton

Merged: sage-4.3.2.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7820

@aghitza aghitza added this to the sage-4.3.2 milestone Jan 2, 2010
@aghitza

This comment has been minimized.

@aghitza
Copy link
Author

aghitza commented Jan 3, 2010

Author: Alex Ghitza

@aghitza
Copy link
Author

aghitza commented Jan 3, 2010

comment:2

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.

@sagetrac-mhampton
Copy link
Mannequin

sagetrac-mhampton mannequin commented Jan 3, 2010

comment:3

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.

@sagetrac-mhampton
Copy link
Mannequin

sagetrac-mhampton mannequin commented Jan 3, 2010

comment:4

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

@sagetrac-mhampton
Copy link
Mannequin

sagetrac-mhampton mannequin commented Jan 4, 2010

comment:5

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.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:6

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

@aghitza
Copy link
Author

aghitza commented Jan 5, 2010

comment:7

Replying to @sagetrac-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.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:8

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:9

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:10

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:11

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

Changed author from Alex Ghitza to Alex Ghitza, David Kirkby

@aghitza
Copy link
Author

aghitza commented Jan 5, 2010

comment:13

Replying to @sagetrac-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.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:14

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.

@aghitza
Copy link
Author

aghitza commented Jan 5, 2010

comment:15

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.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Jan 5, 2010

comment:16

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

@aghitza
Copy link
Author

aghitza commented Jan 6, 2010

apply after the gfan-0.4plus spkg

@aghitza

This comment has been minimized.

@aghitza
Copy link
Author

aghitza commented Jan 6, 2010

comment:17

Attachment: trac_7820.patch.gz

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.

@aghitza
Copy link
Author

aghitza commented Jan 6, 2010

comment:18

Replying to @aghitza:

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.

@aghitza

This comment has been minimized.

@sagetrac-mhampton
Copy link
Mannequin

sagetrac-mhampton mannequin commented Jan 23, 2010

comment:20

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.

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 25, 2010

Reviewer: Marshall Hampton

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 25, 2010

Merged: sage-4.3.2.alpha0

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 25, 2010

comment:21

Merged in this order:

  1. merged gfan-0.4plus.spkg in the standard spkg repository
  2. trac_7820.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant