Opened 5 years ago
Closed 5 years ago
#23847 closed enhancement (fixed)
make the experimental FriCAS package optional
Reported by: | mantepse | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | packages: optional | Keywords: | FriCAS |
Cc: | chapoton, charpent, embray | Merged in: | |
Authors: | Martin Rubey | Reviewers: | Emmanuel Charpentier, Jeroen Demeyer, Dima Pasechnik, Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | f48e8c5 (Commits, GitHub, GitLab) | Commit: | f48e8c5a94d1df496a17189a5b77e513ab053d2d |
Dependencies: | #21377,#22525,#23782 | Stopgaps: |
Description
FriCAS is a CAS that provides some functionality which is otherwise either missing (e.g., guessing formulas), or not as complete as might be desirable (e.g., symbolic integration).
The FriCAS interface was completely rewritten after FriCAS was demoted to experimental in #18563 and is now quite mature, containing lots of doctests.
Change History (61)
comment:1 Changed 5 years ago by
- Keywords FriCAS added
comment:2 follow-up: ↓ 3 Changed 5 years ago by
- Status changed from new to needs_review
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 5 Changed 5 years ago by
comment:4 Changed 5 years ago by
- Cc charpent added
- Status changed from needs_review to needs_info
comment:5 in reply to: ↑ 3 ; follow-up: ↓ 6 Changed 5 years ago by
In your post, you quoted "guessing formulas", lazy powerseries, as well as enhancements to symbolic integration and differential equations solving. Documenting those examples, either in the relevant "main" documentation or in a specific page) and doctesting them would be useful.
These are actually in the main documentation. To see it:
sage: import sage.interfaces.fricas sage: sage.interfaces.fricas?
I agree that exposing some of the functionality would be good, but I think that's for another ticket. I wouldn't do it for an experimental package.
comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 5 years ago by
Replying to mantepse:
[ Snip... ]
I agree that exposing some of the functionality would be good, but I think that's for another ticket.
You may be right...
I wouldn't do it for an experimental package.
But your ticket aims to make it an optional package...
Anyway : sage 8.1.beta6+#23782 passes ptestlong
without any error. What remains to do is changing $SAGE_ROOT/build/pkgs/fricas/type from experimental
to optional
, fix the checksums, commit and push. Care to do it ? (I can review it this afternoon...).
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 5 years ago by
I wouldn't do it for an experimental package.
But your ticket aims to make it an optional package...
Yes, I might do it for an optional package :-)
What remains to do is changing $SAGE_ROOT/build/pkgs/fricas/type from
experimental
tooptional
, fix the checksums, commit and push. Care to do it ? (I can review it this afternoon...).
why would the checksum change? (fricas was updated in #21377)
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 10 Changed 5 years ago by
Replying to mantepse:
I wouldn't do it for an experimental package.
But your ticket aims to make it an optional package...
Yes, I might do it for an optional package :-)
What remains to do is changing $SAGE_ROOT/build/pkgs/fricas/type from
experimental
tooptional
, fix the checksums, commit and push. Care to do it ? (I can review it this afternoon...).why would the checksum change? (fricas was updated in #21377)
'Cause, unless I'm mistaken, the $SAGE_ROOT/build/pkgs/fricas/type file is accoundd for in the checksum computation ?
comment:9 Changed 5 years ago by
It would be easier to test this if all dependencies would actually be merged in a beta release.
comment:10 in reply to: ↑ 8 Changed 5 years ago by
Replying to charpent:
'Cause, unless I'm mistaken, the $SAGE_ROOT/build/pkgs/fricas/type file is accoundd for in the checksum computation ?
No, the checksum is the checksum of the source tarball. It does not depend on anything from Sage.
comment:11 follow-up: ↓ 12 Changed 5 years ago by
About this ticket itself: the way I see it, the only difference between an optional package and an experimental package is that an optional package is required to work on all supported platforms. "working" is defined as building, passing its own testsuite and passing Sage doctests. Regarding the test suite: does FriCAS have one? There is no spkg-check
file.
If it works as defined above, then there doesn't even need to be a vote: just make it optional.
I can run some tests on various systems. As I said above, I'd prefer to wait for a beta release where are dependencies have been merged.
comment:12 in reply to: ↑ 11 Changed 5 years ago by
Replying to jdemeyer:
About this ticket itself: the way I see it, the only difference between an optional package and an experimental package is that an optional package is required to work on all supported platforms. "working" is defined as building, passing its own testsuite and passing Sage doctests.
One of the FriCAS maintainers says (https://groups.google.com/d/msg/fricas-devel/s5WtJx8u-kA/XVJPuUAQBwAJ) that this should be the case. I have no way of testing it.
Regarding the test suite: does FriCAS have one? There is no
spkg-check
file.
Concerning this, he answered:
You can invoke the testsuite by "cd src/input && make", although there's no script to collect the results, there's an unofficial one:
I wonder how useful this testsuite is for sage then. I also do not know how long it takes to run it. I just started it.
If it works as defined above, then there doesn't even need to be a vote: just make it optional.
I can run some tests on various systems. As I said above, I'd prefer to wait for a beta release where are dependencies have been merged.
I'll set the type to optional and the ticket to needs-review and you do it whenever you wish, OK?
comment:13 Changed 5 years ago by
- Branch set to u/mantepse/make_the_experimental_fricas_package_optional
comment:14 follow-up: ↓ 15 Changed 5 years ago by
- Commit set to f8567ebfe1a7f690e069dff95f4ebcacc6b90aca
On my laptop (Intel(R) Celeron(R) CPU N2940 @ 1.83GHz) the test suite runs for about half an hour. For each of the ~ 200 input files it produces an output file. Essential differences mean new bugs.
I don't think that we want to run this.
New commits:
f8567eb | set type to optional
|
comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 5 years ago by
Replying to mantepse:
I don't think that we want to run this.
Why not? I don't understand how you came to this conclusion.
comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 5 years ago by
Replying to jdemeyer:
Replying to mantepse:
I don't think that we want to run this.
Why not? I don't understand how you came to this conclusion.
Because you don't get a result, such as "tests passed". Well, it might make sense to run it and say something like "output of tests can be found in ..."
What do you think?
comment:17 in reply to: ↑ 16 Changed 5 years ago by
comment:18 Changed 5 years ago by
- Status changed from needs_info to needs_review
comment:19 Changed 5 years ago by
- Cc embray added
LGTM : on Debian testing running on core i7 + 16 GB RAM, 8.1-beta7+#23847 passes ptestlong
with no error whatsoever (mirabile dictu), as well as sage -t --optional fricas,sage src/sage/interfaces/fricas.py
.
As suggested by Jeroen Demeyer, I think that this should be tested on other "officially supported Sage platforms", whatever that means.
At the very minimum, I suggest Mac OS X (which is full of surprises, nowadays), the Sage appliance used under Windows, and maybe the Cygwin(-64 ?) port made by Erik Bray, if Erik thinks that his port is ready for diffusion (I put him in Cc
).
Status remains at needs_review
, but should be put as positive_review
when this will have been tested on all "interesting" platforms.
comment:20 follow-up: ↓ 21 Changed 5 years ago by
All dependencies have now been merged in 8.1.beta7
comment:21 in reply to: ↑ 20 Changed 5 years ago by
Replying to jdemeyer:
All dependencies have now been merged in 8.1.beta7
Indeed. I waited for this before ptest
ing it.
comment:22 Changed 5 years ago by
- Reviewers set to Frédéric Chapoton, Jeroen Demeyer
comment:23 Changed 5 years ago by
One thing which is very annoying is that the build cannot be interrupted with CTRL-C
.
comment:24 Changed 5 years ago by
Works fine on Linux i386 (32-bit).
comment:25 Changed 5 years ago by
- Status changed from needs_review to needs_work
On Linux sardonis 4.4.0-57-generic #78-Ubuntu SMP Fri Dec 9 23:46:13 UTC 2016 ppc64le ppc64le ppc64le GNU/Linux
:
sage -t --long src/sage/interfaces/fricas.py ********************************************************************** File "src/sage/interfaces/fricas.py", line 69, in sage.interfaces.fricas Failed example: f^2 # optional - fricas Expected: 10 5 5 10 y - 2x y + x Got: 10 5 5 10 y - 2 x y + x ********************************************************************** File "src/sage/interfaces/fricas.py", line 95, in sage.interfaces.fricas Failed example: fricas("guessADE([partition n for n in 0..40], homogeneous==4)") # optional - fricas Expected: [ [ n [x ]f(x): 2 3 (iv) 2 2 , 3 ,,, 2 2 ,, 2 x f(x) f (x) + (20x f(x) f (x) + 5x f(x) )f (x) - 39x f(x) f (x) <BLANKLINE> + 2 , 2 2 , 3 ,, 2 , 4 (12x f(x)f (x) - 15x f(x) f (x) + 4f(x) )f (x) + 6x f (x) <BLANKLINE> + , 3 2 , 2 10x f(x)f (x) - 16f(x) f (x) <BLANKLINE> = 0 , 2 3 4 f(x)= 1 + x + 2x + 3x + O(x )] ] Got: [ [ n [x ]f(x): 2 3 (iv) 2 2 , 3 ,,, x f(x) f (x) + (20 x f(x) f (x) + 5 x f(x) )f (x) + 2 2 ,, 2 - 39 x f(x) f (x) + 2 , 2 2 , 3 ,, 2 , 4 (12 x f(x)f (x) - 15 x f(x) f (x) + 4 f(x) )f (x) + 6 x f (x) + , 3 2 , 2 10 x f(x)f (x) - 16 f(x) f (x) = 0 , 2 3 4 f(x) = 1 + x + 2 x + 3 x + O(x )] ] ********************************************************************** File "src/sage/interfaces/fricas.py", line 122, in sage.interfaces.fricas Failed example: fricas.set("sol", "solve(deq, y, x)"); fricas("sol") # optional - fricas Expected: 5 3 2 3 2 3 3 2 x - 10x + 20x + 4 2x - 3x + 1 x - 1 x - 3x - 1 [particular= --------------------,basis= [-------------,------,------------]] 15x x x x Got: 5 3 2 x - 10 x + 20 x + 4 [particular = ----------------------, 15 x 3 2 3 3 2 2 x - 3 x + 1 x - 1 x - 3 x - 1 basis = [---------------, ------, -------------]] x x x ********************************************************************** File "src/sage/interfaces/fricas.py", line 157, in sage.interfaces.fricas Failed example: 2*(1+2*x+sqrt(1-4*x)-2*x*y).recip() # optional - fricas Expected: 2 3 2 2 3 4 4 5 1 + (x y + x ) + 2x + (x y + 2x y + 6x ) + (4x y + 18x ) + 3 3 4 2 5 6 5 2 6 7 (x y + 3x y + 13x y + 57x ) + (6x y + 40x y + 186x ) + 4 4 5 3 6 2 7 8 6 3 7 2 8 9 (x y + 4x y + 21x y + 130x y + 622x ) + (8x y + 66x y + 432x y + 2120x ) + 5 5 6 4 7 3 8 2 9 10 (x y + 5x y + 30x y + 220x y + 1466x y + 7338x ) + O(11) Got: 2 3 2 2 3 4 4 5 1 + (x y + x ) + 2 x + (x y + 2 x y + 6 x ) + (4 x y + 18 x ) + 3 3 4 2 5 6 5 2 6 7 (x y + 3 x y + 13 x y + 57 x ) + (6 x y + 40 x y + 186 x ) + 4 4 5 3 6 2 7 8 (x y + 4 x y + 21 x y + 130 x y + 622 x ) + 6 3 7 2 8 9 (8 x y + 66 x y + 432 x y + 2120 x ) + 5 5 6 4 7 3 8 2 9 10 (x y + 5 x y + 30 x y + 220 x y + 1466 x y + 7338 x ) + O(11) ********************************************************************** File "src/sage/interfaces/fricas.py", line 543, in sage.interfaces.fricas.FriCAS.get Failed example: fricas.get(a.name()) # optional - fricas Expected: ' +-+\r\n29\\|2 + 41' Got: ' +-+\r\n29 \\|2 + 41' ********************************************************************** File "src/sage/interfaces/fricas.py", line 545, in sage.interfaces.fricas.FriCAS.get Failed example: fricas.get('(1 + sqrt(2))^5') # optional - fricas Expected: ' +-+\r\n29\\|2 + 41' Got: ' +-+\r\n29 \\|2 + 41' ********************************************************************** File "src/sage/interfaces/fricas.py", line 547, in sage.interfaces.fricas.FriCAS.get Failed example: fricas.new('(1 + sqrt(2))^5') # optional - fricas Expected: +-+ 29\|2 + 41 Got: +-+ 29 \|2 + 41 ********************************************************************** File "src/sage/interfaces/fricas.py", line 629, in sage.interfaces.fricas.FriCAS.get_unparsed_InputForm Failed example: fricas.get_unparsed_InputForm('1..3') # optional - fricas Expected: '1..3$Segment(Integer())' Got: '(1..3)$Segment(PositiveInteger())' ********************************************************************** File "src/sage/interfaces/fricas.py", line 655, in sage.interfaces.fricas.FriCAS._equality_symbol Failed example: a = fricas(x==6); a # optional - fricas, indirect doctest Expected: x= 6 Got: x = 6 ********************************************************************** File "src/sage/interfaces/fricas.py", line 661, in sage.interfaces.fricas.FriCAS._equality_symbol Failed example: a = fricas(x==6); a # optional - fricas Expected: 2= 6 Got: 2 = 6 ********************************************************************** File "src/sage/interfaces/fricas.py", line 1147, in sage.interfaces.fricas.FriCASElement._sage_ Failed example: a = fricas('(1 + sqrt(2))^5'); a # optional - fricas Expected: +-+ 29\|2 + 41 Got: +-+ 29 \|2 + 41 **********************************************************************
comment:26 Changed 5 years ago by
Sorry, I just realized now that I should rebase to 8.1.beta7
comment:27 Changed 5 years ago by
- Branch changed from u/mantepse/make_the_experimental_fricas_package_optional to u/jdemeyer/make_the_experimental_fricas_package_optional
comment:28 Changed 5 years ago by
- Commit changed from f8567ebfe1a7f690e069dff95f4ebcacc6b90aca to 6409c5b2cbdfe454899b6ed8bbe1d4b314855533
- Status changed from needs_work to needs_review
comment:29 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:30 Changed 5 years ago by
Did Frédéric do some of the review? It seems to me that it was Emmanuel Charpentier that should be listed as co-reviewer.
comment:31 Changed 5 years ago by
- Reviewers changed from Frédéric Chapoton, Jeroen Demeyer to Emmanuel Charpentier, Jeroen Demeyer
Of course. I confused the user names "charpent" and "chapoton".
comment:32 Changed 5 years ago by
I will test now on Cygwin.
comment:33 Changed 5 years ago by
- Status changed from positive_review to needs_work
[fricas-1.3.2] Thread model: posix [fricas-1.3.2] gcc version 5.4.0 (GCC) [fricas-1.3.2] **************************************************** [fricas-1.3.2] checking build system type... x86_64-cygwin [fricas-1.3.2] configure: error: invalid value of canonical build [fricas-1.3.2] Error configuring fricas. [fricas-1.3.2] [fricas-1.3.2] real 0m1.548s [fricas-1.3.2] user 0m0.347s [fricas-1.3.2] sys 0m1.101s
I'll investigate. I have some suspicions...
comment:34 follow-up: ↓ 38 Changed 5 years ago by
As I suspected, the problem is a buggy/outdated config.guess
. I'm not sure how we want to go about fixing that. The simplest way is to just include an up-to-date version and copy it over...
comment:35 follow-ups: ↓ 36 ↓ 39 Changed 5 years ago by
Weirdly, last time FriCAS updated their `config.guess`, the version it was updated to does not correspond exactly with the upstream `config.guess` from the same date, almost as if it were hand-edited and mistakes were made.
comment:36 in reply to: ↑ 35 ; follow-up: ↓ 46 Changed 5 years ago by
I forwarded your observation to the FriCAS mailing list...
comment:37 Changed 5 years ago by
It's also strange that they updated that file in 2017 to a version from 2014.
comment:38 in reply to: ↑ 34 Changed 5 years ago by
comment:39 in reply to: ↑ 35 Changed 5 years ago by
Replying to embray:
almost as if it were hand-edited and mistakes were made.
It could be that they used a version from a (Linux) distribution, which might ship a patched version. That might also explain the discrepancy in the year, if the latest version shipped by that distro is from 2014.
comment:40 Changed 5 years ago by
- Commit changed from 6409c5b2cbdfe454899b6ed8bbe1d4b314855533 to e765d16e3a13600a0dcb28f109c4bb3d046fed4d
Branch pushed to git repo; I updated commit sha1. New commits:
e765d16 | Use newer version of config.guess and config.sub
|
comment:41 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:42 Changed 5 years ago by
- Status changed from needs_review to needs_work
sage -t --long src/sage/interfaces/fricas.py ********************************************************************** File "src/sage/interfaces/fricas.py", line 313, in sage.interfaces.fricas.FriCAS._quit_string Failed example: pr.children() # optional - fricas Expected: [<psutil.Process(pid=..., name='AXIOMsys') at ...>, <psutil.Process(pid=..., name='session') at ...>, <psutil.Process(pid=..., name='spadclient') at ...>, <psutil.Process(pid=..., name='sman') at ...>] Got: [<psutil.Process(pid=9800, name='session') at 7660406796560>, <psutil.Process(pid=19536, name='spadclient') at 7660406797648>, <psutil.Process(pid=21068, name='AXIOMsys') at 7660406797712>, <psutil.Process(pid=21532, name='sman') at 7660406797776>]
It seems psutil is sorting the processes by pid, so there's no way to guarantee what order they're in. I don't know what this test is for or if it's important but if it really is then it should sort these processes by name.
comment:43 Changed 5 years ago by
(Build works for me now; thanks)
comment:44 Changed 5 years ago by
This test is part of an effort to make sure that FriCAS is really shut down when quitting. This is actually an unsolved problem, it doesn't work reliably. Taking into account your observation, the following would make it pass:
sage: import psutil # optional - fricas sage: p = fricas.pid(); pr = psutil.Process(p); pr # optional - fricas <psutil.Process(pid=..., name='sman') at ...> sage: sorted(pr.children(), key = lambda c: c.name()) # optional - fricas [<psutil.Process(pid=..., name='AXIOMsys') at ...>, <psutil.Process(pid=..., name='session') at ...>, <psutil.Process(pid=..., name='spadclient') at ...>, <psutil.Process(pid=..., name='sman') at ...>] sage: fricas.quit() # optional - fricas sage: pr.is_running() # optional - fricas, random False
However, note that tag random
in the last line - it turns out that frequently, FriCAS doesn't really stop - only the child processes are gone. Actually, we could append
sage: pr.children() if pr.is_running() # optional - fricas []
as final test.
comment:45 Changed 5 years ago by
As it turns out, also that test fails. When doctesting, after fricas.quit()
only some processes are gone immediately.
comment:46 in reply to: ↑ 36 Changed 5 years ago by
Replying to mantepse:
I forwarded your observation to the FriCAS mailing list...
If you have a link to that post, it would be good to put it on this ticket.
comment:47 Changed 5 years ago by
I notified upstream, maybe we should remind them before they make the next release:
https://groups.google.com/d/msg/fricas-devel/cmhTjr8IxPU/tY_Idyf2AQAJ
I am currently recompiling sage, to try the suggestion to start fricas with -nosman
.
comment:48 Changed 5 years ago by
For some reason, using -nosman
doesn't work. I can start fricas on the command line with fricas -nosman
, and in sage manually, but for some reason it breaks the interface :-(
comment:49 Changed 5 years ago by
The issue is a difference in newlines: on linux, with fricas -nosman
, I have
sage: fricas.eval("1+1", reformat=False) '\n|startAlgebraOutput|\n 2\n|endOfAlgebraOutput|'
whereas with fricas -nox -noclef
, I have
sage: fricas.eval("1+1", reformat=False) '\r\n|startAlgebraOutput|\r\n 2\r\n|endOfAlgebraOutput|\r'
I'm not sure what the right fix is. I think I'll replace \r\n
throughout with \n
.
comment:50 Changed 5 years ago by
- Branch changed from u/jdemeyer/make_the_experimental_fricas_package_optional to u/mantepse/make_the_experimental_fricas_package_optional
- Commit changed from e765d16e3a13600a0dcb28f109c4bb3d046fed4d to f8567ebfe1a7f690e069dff95f4ebcacc6b90aca
New commits:
f8567eb | set type to optional
|
comment:51 Changed 5 years ago by
- Branch u/mantepse/make_the_experimental_fricas_package_optional deleted
- Commit f8567ebfe1a7f690e069dff95f4ebcacc6b90aca deleted
comment:52 Changed 5 years ago by
- Branch set to u/jdemeyer/make_the_experimental_fricas_package_optional
- Commit set to e765d16e3a13600a0dcb28f109c4bb3d046fed4d
comment:53 Changed 5 years ago by
Apparently, I cannot push anymore. Well, it's working now, I'll try to attach a patch.
comment:54 Changed 5 years ago by
- Branch changed from u/jdemeyer/make_the_experimental_fricas_package_optional to u/mantepse/make_the_experimental_fricas_package_optional
- Commit changed from e765d16e3a13600a0dcb28f109c4bb3d046fed4d to f48e8c5a94d1df496a17189a5b77e513ab053d2d
comment:55 Changed 5 years ago by
- Status changed from needs_work to needs_review
should work now! please test on cygwin and MacOS X - I modified the handling of newlines!
comment:56 Changed 5 years ago by
tested on FreeBSD 11 with clang (thus pretty close to OSX) (cf #22679), all tests in src/sage/interfaces/fricas.py
pass.
comment:57 Changed 5 years ago by
ping?
comment:58 Changed 5 years ago by
I'll test on OSX later today.
comment:59 Changed 5 years ago by
- Reviewers changed from Emmanuel Charpentier, Jeroen Demeyer to Emmanuel Charpentier, Jeroen Demeyer, Dima Pasechnik
- Status changed from needs_review to positive_review
OK, all good on OSX.
comment:60 Changed 5 years ago by
- Reviewers changed from Emmanuel Charpentier, Jeroen Demeyer, Dima Pasechnik to Emmanuel Charpentier, Jeroen Demeyer, Dima Pasechnik, Erik Bray
LGTM now that that one test is fixed.
comment:61 Changed 5 years ago by
- Branch changed from u/mantepse/make_the_experimental_fricas_package_optional to f48e8c5a94d1df496a17189a5b77e513ab053d2d
- Resolution set to fixed
- Status changed from positive_review to closed
Replying to mantepse:
What kind of review do you think is suitable for this ? I have had fricas installed for about 2 years, with no specific problems.
Since, AFAICT, #21377 and #22525 are already merged in 8.1.beta6, the only test I could do would be to test #23782 and
ptestlong
.Maybe we should grasp the opportunity to document this interface a bit better, and possibly doctest some opportunities made available by fricas ?
In your post, you quoted "guessing formulas", lazy powerseries, as well as enhancements to symbolic integration and differential equations solving. Documenting those examples, either in the relevant "main" documentation or in a specific page) and doctesting them would be useful.
What do you think ?