Opened 2 years ago
Closed 2 years ago
#29552 closed enhancement (fixed)
Upgrade giac to 1.5.087
Reported by:  mkoeppe  Owned by:  

Priority:  blocker  Milestone:  sage9.2 
Component:  packages: standard  Keywords:  giac 
Cc:  arojas, dimpase, frederichan, parisse, slelievre, infinity0, saraedum, thansen, ghtimokau, fbissey, ghmwageringel  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  merge #29171 
Branch:  eb7995f (Commits, GitHub, GitLab)  Commit:  eb7995f997cb53baa572bfd8543dfb2b887bcf2e 
Dependencies:  #29521, #29171  Stopgaps: 
Attachments (2)
Change History (47)
comment:1 Changed 2 years ago by
 Dependencies set to #29521
comment:2 Changed 2 years ago by
comment:3 followup: ↓ 4 Changed 2 years ago by
For 1.6 with laplace I have reported it upstream on giac's forum: https://xcas.univgrenoblealpes.fr/forum/viewtopic.php?f=3&t=2531
but for the sqrt(5) appearing in the doctests of giac.py I suspect some default configuration that is not dependent of the version of giac. Could it be that the user .xcasrc is taken into account?
Then I think it would be more natural to force the configuration to not use sqrt before the computation with:
sage: giac.with_sqrt(0);
NB: this function returns the previous value of this setting so in a doctest it is better to not print the answer)
comment:4 in reply to: ↑ 3 Changed 2 years ago by
Replying to frederichan:
but for the sqrt(5) appearing in the doctests of giac.py I suspect some default configuration that is not dependent of the version of giac. Could it be that the user .xcasrc is taken into account?
downgrading giac makes those tests pass again, so there's definitely some change in 1.6 involved.
comment:5 Changed 2 years ago by
Yes, it's an improvement, done for geogebra. Forcing the sqrt mode of Xcas to false should fix sage regression test, as explained by Frederic. Laplace bug is fixed in my source tree, I will make a new release probably next week.
diff desolve.cc desolve.cc~ 117c117 < if (lop(res,at_integrate).empty() && lop(res,at_piecewise).empty() && lop(res,at_sign).empty())  > if (lop(res,at_integrate).empty())
comment:6 Changed 2 years ago by
 Cc ghmwageringel added
comment:7 Changed 2 years ago by
Incidentally, with giac 1.5.063 on manylinux2010standard
(from #30195), I get (https://github.com/kliem/sage/runs/908011984)
In file included from misc.cc:9275: misc.cc: In function 'std::string giac::fetch(const string&)': misc.cc:9289:28: error: 'CURLOPT_ACCEPT_ENCODING' was not declared in this scope curl_easy_setopt(curl, CURLOPT_ACCEPT_ENCODING, "deflate"); ^~~~~~~~~~~~~~~~~~~~~~~ misc.cc:9289:28: note: suggested alternative: 'CURLOPT_ENCODING'
It should be checked whether this goes away with the update
comment:8 Changed 2 years ago by
Yes, I do remember a problem with curl in an earlier giac. I believe it is fixed in 1.5.087+.
comment:9 Changed 2 years ago by
Should we continue waiting for another 1.6 patch release or upgrade to latest 1.5.x in the meantime?
comment:10 Changed 2 years ago by
I would be OK to move to 1.5.087 already.
comment:11 Changed 2 years ago by
 Branch set to u/mkoeppe/upgrade_giac_to_1_5_0_87
comment:12 Changed 2 years ago by
 Commit set to d2324a69aee28beb276fb7a33bf9475f674e618e
New commits:
d2324a6  build/pkgs/giac: Update to 1.5.0.87

Changed 2 years ago by
comment:13 Changed 2 years ago by
 Reviewers set to https://github.com/mkoeppe/sage/actions/runs/202114883
 Status changed from new to needs_review
Builds OK on macOS
comment:14 followup: ↓ 17 Changed 2 years ago by
You'll need https://github.com/cschwan/sageongentoo/blob/master/scimathematics/sage/files/giac1.5.0.87.patch for sage's doctests.
comment:15 Changed 2 years ago by
Updating doctests is probably better done together with #29541
comment:16 Changed 2 years ago by
By the way, it seems that we don't install giac correctly (before or after this update):
$ giac help // Maximum number of parallel threads 1 // Unable to find keyword file /Applications/usr/share/giac/doc/en/keywords Help file /Applications/usr/share/giac/doc/en/aide_cas not found Added 0 synonyms Help file /Applications/usr/share/giac/aide_cas not found :1: syntax error line 1 col 1 at  in help undef // dclock2 0.00015 // Time 0 // Total time 0
comment:17 in reply to: ↑ 14 ; followup: ↓ 19 Changed 2 years ago by
Replying to fbissey:
You'll need https://github.com/cschwan/sageongentoo/blob/master/scimathematics/sage/files/giac1.5.0.87.patch for sage's doctests.
Let's see...
diff git a/sage/interfaces/giac.py b/sage/interfaces/giac.py index 68c9527..3c5fd12 100644  a/sage/interfaces/giac.py +++ b/sage/interfaces/giac.py @@ 990,7 +990,7 @@ class GiacElement(ExpectElement): EXAMPLES:: sage: print(latex(giac('(x^4  y)/(y^23*x)')))  "\frac{(x^{4}y)}{(y^{2}3\cdot x)}" + "\frac{x^{4}y}{y^{2}3 x}" """ return self.parent().eval('latex(%s)'%self.name())
Before or after, this doctest makes no sense.
The _latex_
method is broken  it adds quotation marks
sage: g = giac('(x^4  y)/(y^23*x)') sage: g (x^4y)/(y^23*x) sage: s = g._sage_() sage: s (x^4  y)/(y^2  3*x) sage: latex(s) \frac{x^{4}  y}{y^{2}  3 \, x} sage: latex(g) "\frac{(x^{4}y)}{(y^{2}3\cdot x)}"
Is anyone using this interface at all?
comment:18 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:19 in reply to: ↑ 17 Changed 2 years ago by
Replying to mkoeppe:
Replying to fbissey:
You'll need https://github.com/cschwan/sageongentoo/blob/master/scimathematics/sage/files/giac1.5.0.87.patch for sage's doctests.
Let's see...
diff git a/sage/interfaces/giac.py b/sage/interfaces/giac.py index 68c9527..3c5fd12 100644  a/sage/interfaces/giac.py +++ b/sage/interfaces/giac.py @@ 990,7 +990,7 @@ class GiacElement(ExpectElement): EXAMPLES:: sage: print(latex(giac('(x^4  y)/(y^23*x)')))  "\frac{(x^{4}y)}{(y^{2}3\cdot x)}" + "\frac{x^{4}y}{y^{2}3 x}" """ return self.parent().eval('latex(%s)'%self.name())Before or after, this doctest makes no sense.
The
_latex_
method is broken  it adds quotation markssage: g = giac('(x^4  y)/(y^23*x)') sage: g (x^4y)/(y^23*x) sage: s = g._sage_() sage: s (x^4  y)/(y^2  3*x) sage: latex(s) \frac{x^{4}  y}{y^{2}  3 \, x} sage: latex(g) "\frac{(x^{4}y)}{(y^{2}3\cdot x)}"Is anyone using this interface at all?
I don't know :)  the pexpect interface and the optional giacpy_sage package are the only way to interact with giac right now. I cannot find anything linked with libgiac in sagelib. But the latex interface in particular? I don't know. The latex spat out for that doctest is different in the currently shipped giac, 1.5.063 and 1.5.087.
comment:20 Changed 2 years ago by
 Commit changed from d2324a69aee28beb276fb7a33bf9475f674e618e to 0971326e7088a5bbc0ceb11cefb60a4f40d0ae1b
Branch pushed to git repo; I updated commit sha1. New commits:
0971326  sage.interfaces.giac.GiacElement._latex_: Fix implementation and doctest, make doctest more flexible

comment:21 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:22 Changed 2 years ago by
 Commit changed from 0971326e7088a5bbc0ceb11cefb60a4f40d0ae1b to 8bbd05637c0b4a2d7a733af099540c91805363e2
comment:23 Changed 2 years ago by
 Reviewers changed from https://github.com/mkoeppe/sage/actions/runs/202114883 to github.com/mkoeppe/sage/actions/runs/203238491
comment:24 Changed 2 years ago by
 Commit changed from 8bbd05637c0b4a2d7a733af099540c91805363e2 to c076383f03d7bc22d98a0a7b442f3e7b877799a0
Branch pushed to git repo; I updated commit sha1. New commits:
c076383  src/sage/interfaces/giac.py: Make new latex doctest more flexible

comment:25 Changed 2 years ago by
Tests at https://github.com/mkoeppe/sage/actions/runs/203238491:
Clean on ubuntutrustystandard
(https://github.com/mkoeppe/sage/runs/969005090) and fedora32standard
(https://github.com/mkoeppe/sage/runs/969005556) except for doctest fixed in c076383.
comment:26 Changed 2 years ago by
 Reviewers changed from github.com/mkoeppe/sage/actions/runs/203238491 to github.com/mkoeppe/sage/actions/runs/203238491, ...
comment:27 Changed 2 years ago by
Needs review
comment:28 Changed 2 years ago by
there is a strange behavior in this version of giac (I have tested with 1.5.0.85, 1.5.0.87 and 1.6.0.7)that breaks giacpy_sage doctests. (cf your comment 25 in #29171) indeed the c++ _size function applied to the empty list seq[] doesn't return 0 as with giac 1.5.0.63 and earlier version but it returns some configuration setting.
Example: in icas len(seq[])
gives 0 with 1.5.0.63 but it now gives
[["atom","cplx","idnt","frac","vector","symb","strng","other","total"],[2,0,0,0,3,0,0,0,240]]
with 1.5.0.87
I have reported it there https://xcas.univgrenoblealpes.fr/forum/viewtopic.php?f=3&t=2534#p11776 to know if this modification will remain or is a bug to know if we patch giac or adapt giacpy.
comment:29 Changed 2 years ago by
 Status changed from needs_review to needs_info
OK, I guess we should hold off with this update.
comment:30 Changed 2 years ago by
So the answer was that it is a modification usefull for very low memory systems (calculators). Whether or not these extra lines will be put inside an #ifdef in later versions of giac is still in discussion. I guess we can let it as is
comment:31 Changed 2 years ago by
 Dependencies changed from #29521 to #29521, #29171
comment:32 Changed 2 years ago by
 Status changed from needs_info to needs_work
 Work issues set to merge #29171
comment:33 Changed 2 years ago by
For #30351, we should apply
https://github.com/cschwan/sageongentoo/blob/master/scimathematics/giac/files/giac1.5.0.87gsl_lapack.patch
in spkgsrc
.
comment:34 Changed 2 years ago by
 Commit changed from c076383f03d7bc22d98a0a7b442f3e7b877799a0 to eb7995f997cb53baa572bfd8543dfb2b887bcf2e
Changed 2 years ago by
comment:35 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:36 Changed 2 years ago by
 Reviewers changed from github.com/mkoeppe/sage/actions/runs/203238491, ... to github.com/mkoeppe/sage/actions/runs/211389096, github.com/mkoeppe/sage/actions/runs/211389095, ...
comment:37 Changed 2 years ago by
 Priority changed from major to blocker
comment:38 Changed 2 years ago by
 Keywords giac added
comment:39 Changed 2 years ago by
Needs review!
comment:40 followup: ↓ 42 Changed 2 years ago by
$ giac // Maximum number of parallel threads 1 Help file /Applications/usr/share/giac/aide_cas not found // Unable to find keyword file /Applications/usr/share/giac/doc/en/keywords Help file /Applications/usr/share/giac/doc/en/aide_cas not found Added 0 synonyms Welcome to giac readline interface (c) 2001,2018 B. Parisse & others Homepage http://wwwfourier.ujfgrenoble.fr/~parisse/giac.html Released under the GPL license 3.0 or above See http://www.gnu.org for license details May contain BSD licensed software parts (lapack, atlas, tinymt)  Press CTRL and D simultaneously to finish session Type ?commandname for help 0>>
something with keywords ? (This is on macOS 10.15).
comment:41 Changed 2 years ago by
but linking to openblas is fixed, good.
comment:42 in reply to: ↑ 40 Changed 2 years ago by
Replying to dimpase:
$ giac // Maximum number of parallel threads 1 Help file /Applications/usr/share/giac/aide_cas not found // Unable to find keyword file /Applications/usr/share/giac/doc/en/keywords Help file /Applications/usr/share/giac/doc/en/aide_cas not found Added 0 synonyms Welcome to giac readline interface (c) 2001,2018 B. Parisse & others Homepage http://wwwfourier.ujfgrenoble.fr/~parisse/giac.html Released under the GPL license 3.0 or above See http://www.gnu.org for license details May contain BSD licensed software parts (lapack, atlas, tinymt)  Press CTRL and D simultaneously to finish session Type ?commandname for help 0>>something with keywords ? (This is on macOS 10.15).
Frederic indicates that these messages are not error messages. See https://trac.sagemath.org/ticket/29171#comment:47
comment:43 Changed 2 years ago by
Moreover, for the keyword message, I have tried on linux with parisse's debian package. with LANG=C I have also the same (0 synonyms added) with LANG=en_US I have 26 synonyms added but all are GUI related (turtle mode). so I don't think that it is a problem. so this ticket looks good to me. tested on fedora fc30.x86_64 (make test was OK)
comment:44 Changed 2 years ago by
 Reviewers changed from github.com/mkoeppe/sage/actions/runs/211389096, github.com/mkoeppe/sage/actions/runs/211389095, ... to Dima Pasechnik
 Status changed from needs_review to positive_review
ok
comment:45 Changed 2 years ago by
 Branch changed from u/mkoeppe/upgrade_giac_to_1_5_0_87 to eb7995f997cb53baa572bfd8543dfb2b887bcf2e
 Resolution set to fixed
 Status changed from positive_review to closed
1.6.0 is out, causing a few test failures:
The ones in
interfaces/giac.py
just need the tests updated, thecalculus/calculus.py
failures are an upstream issue