janverschelde / PHCpack

The primary source code repository for PHCpack, a software package to solve polynomial systems with homotopy continuation methods.
http://www.phcpack.org
GNU General Public License v3.0
59 stars 21 forks source link

phc v2.4.52 10 Apr 2018: timeout when used with sage 8.0 or 8.1 #23

Open DrXyzzy opened 6 years ago

DrXyzzy commented 6 years ago

After installing phc binary from PHCpack download page and attempting the first example from Sage docs here, we see the following:

sage: from sage.interfaces.phc import phc
sage: R.<x,y> = PolynomialRing(CDF,2)
sage: testsys = [x^2 + 1, x*y - 1]
sage: phc.mixed_volume(testsys)
...
TIMEOUT: Timeout exceeded.
<pexpect.pty_spawn.spawn object at 0x277082cd0>
command: /Users/hal/bin/phc
args: ['/Users/hal/bin/phc', '-m']
buffer (last 100 chars): ' 2, 3, or 4 to select, eventually preceded by i for info : Invalid alternative.  Please try again : '
before (last 100 chars): ' 2, 3, or 4 to select, eventually preceded by i for info : Invalid alternative.  Please try again : '
after: <class 'pexpect.exceptions.TIMEOUT'>
match: None
match_index: None
exitstatus: None
flag_eof: False
pid: 53523
child_fd: 14
closed: False
timeout: 30
delimiter: <class 'pexpect.exceptions.EOF'>
logfile: None
logfile_read: None
logfile_send: None
maxread: 2000
ignorecase: False
searchwindowsize: None
delaybeforesend: 0.05
delayafterclose: 0.1
delayafterterminate: 0.1
searcher: searcher_re:
    0: re.compile("results")

This is seen on Linux Ubuntu 16.04 with the 64-bit Linux phc binary using both sage-8.0 and sage-8.1 in CoCalc sagews and in command terminal, as well as on OS X running sage-8.1 in a terminal with the current macOS phc binary.

Would it be possible to download an earlier version of phc for 64-bit Linux to try to isolate the problem and offer a possible workaround? For example, according to this thread from sage-support, the sage interface to PHCpack was working in July of 2017.

janverschelde commented 6 years ago

Thanks for reporting this issue.

At https://github.com/sagemath/sagelib/blob/master/sage/interfaces/phc.py it may be better to replace line 712 in the function mixed_volume: output_filename = self._output_from_command_list(['phc -m','4','n','n','n'], polys, verbose = verbose) by output_filename = self._output_from_command_list(['phc -m','4','0','n','n'], polys, verbose = verbose)

Polyhedral homotopies with double double and quad double arithmetic were introduced in version 2.3.90 (30 July 2014), which changed one question prompting for a number (zero in this case) instead of y/n.

slel commented 6 years ago

Note that https://github.com/sagemath/sagelib is a snapshot of the state of the Sage library in July 2012.

The change suggested by @janverschelde was made, see line 760 in https://github.com/sagemath/sage/blob/master/src/sage/interfaces/phc.py#L760

slel commented 6 years ago

The best known effort of combining SageMath and PHCPack (also with SnapPy, Regina, flipper, curver, and more) is the "computop-sage" Docker image (where computop stands for computational topology).

The dockerfile reveals computop-sage is currently based on Ubuntu 16.04 and uses PHCpack 2.4.43 (see line 73 of the dockerfile), which was released 2017-08-20, according to

DrXyzzy commented 6 years ago

Thanks @janverschelde and @slel. Line 760 at the link provided by Samuel still has the old code, i.e. 'n' instead of '0' in the third element of the first argument to self._output_from_command_list. Also, neither of the versions of phc.py seems to include the tmp_filename() update in sagemath trac 4411.

If I manually apply both the change suggested by @janverschelde and the fix in trac 4411 to https://github.com/sagemath/sagelib/blob/master/sage/interfaces/phc.py, I am able to run the 3-line demo successfully.

slel commented 6 years ago

@DrXyzzy you are right, the change from 'n' to '0' was not made. But I am not sure what you mean about SageMath trac ticket 4411, since the patch "trac_4411-tmpfilename.patch" was applied, see:

Do you mean you undo that patch? Or apply another patch proposed in SageMath trac ticket 4411, that did not end up being applied?