Opened 15 months ago
Closed 15 months ago
#28101 closed defect (fixed)
update giac to 1.5.063
Reported by:  dimpase  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  packages: standard  Keywords:  
Cc:  embray, fbissey, parisse  Merged in:  
Authors:  Dima Pasechnik  Reviewers:  François Bissey 
Report Upstream:  Fixed upstream, in a later stable release.  Work issues:  
Branch:  132f560 (Commits)  Commit:  132f5606bae42317f6643737f47700e4031f22d5 
Dependencies:  Stopgaps: 
Description (last modified by )
update giac to the latest version  probably related to the problem on #27385
from the source at wwwfourier.ujfgrenoble.fr/~parisse/debian/dists/stable/main/source/giac_1.5.063.*
produced tarball: http://users.ox.ac.uk/~coml0531/sage/giac1.5.0.63.tar.bz2
Change History (28)
comment:1 Changed 15 months ago by
 Description modified (diff)
comment:2 Changed 15 months ago by
 Description modified (diff)
comment:3 Changed 15 months ago by
 Branch set to u/dimpase/packages/giac15061
 Commit set to 277e1d9c0bfda86ed92be31d5f9641a420825475
comment:4 Changed 15 months ago by
 Cc embray fbissey added
 Status changed from new to needs_review
comment:5 Changed 15 months ago by
 Status changed from needs_review to needs_work
I meant to post something about that.
This release in particular breaks the building of giacpy_sage
the following patch https://github.com/cschwan/sageongentoo/blob/master/scimathematics/giac/files/giac1.5.0.61tex_header.patch fixes it. You are welcome to forward it upstream. Note that this is the only instance of #include <tex.h>
all the others (and there is number of them) are "tex.h"
. I don't think that's particularly good style but it works.
For a few releases of giac
a giacpy_sage
doctest has been broken.
sage t long /usr/lib64/python2.7/sitepackages/sage/libs/giac.py ********************************************************************** File "/usr/lib64/python2.7/sitepackages/sage/libs/giac.py", line 209, in sage.libs.giac.? Failed example: B1 = gb_giac(I.gens(),1e16) # optional  giacpy_sage, long time (1s) Expected: Running a probabilistic check for the reconstructed Groebner basis. If successfull, error probability is less than 1e16 ... Got: 0.338649 adding reconstructed ideal generators 18 (reconpart 0.233766 prev 0 augment 0.2 recon_count 8 th 1 recon_n2 18 V[i] 77) 0.339567 # new ideal generators 23 0.567448 adding reconstructed ideal generators 36 (reconpart 0.467532 prev 0.233766 augment 0.2 recon_count 17 th 1 recon_n2 36 V[i] 77) 0.57095 # new ideal generators 41 0.725683 adding reconstructed ideal generators 52 (reconpart 0.675325 prev 0.467532 augment 0.2 recon_count 11 th 1 recon_n2 52 V[i] 77) 0.729126 # new ideal generators 57 Running a probabilistic check for the reconstructed Groebner basis. If successfull, error probability is less than 1e16 and is estimated to be less than 10^63. Use proba_epsilon:=0 to certify (this takes more time). // Groebner basis computation time 0.680109 Memory 212.216M **********************************************************************
Just a lot more verbose.
comment:6 Changed 15 months ago by
 Commit changed from 277e1d9c0bfda86ed92be31d5f9641a420825475 to 1fda15e3882261d366bcc8eb2940130c77727962
Branch pushed to git repo; I updated commit sha1. New commits:
1fda15e  use "tex.h" rather than <tex.h> in #include

comment:7 Changed 15 months ago by
 Report Upstream changed from N/A to Reported upstream. No feedback yet.
 Status changed from needs_work to needs_review
comment:8 Changed 15 months ago by
 Status changed from needs_review to needs_work
What about the giacpy_sage
doctest?
comment:9 Changed 15 months ago by
 Commit changed from 1fda15e3882261d366bcc8eb2940130c77727962 to 0be07f536d21b0c1cc2cabb72ce9fab398a8a6ee
Branch pushed to git repo; I updated commit sha1. New commits:
0be07f5  more verbosity in a long optional test

comment:10 Changed 15 months ago by
 Status changed from needs_work to needs_review
OK, done (I forgot it's a long test that's broken...)
comment:11 Changed 15 months ago by
 Reviewers set to François Bissey
 Status changed from needs_review to positive_review
LGTM now.
comment:12 Changed 15 months ago by
 Status changed from positive_review to needs_info
I can confirm that this update seems to solve the problem I described in #27385 on Linux. If I understand correctly, this should also solve the problem on Cygwin. If so, the cygwinicas.patch
should be deleted, should it not?
comment:13 Changed 15 months ago by
 Status changed from needs_info to positive_review
Unless it is tested on Cygwin, I won't rush. It's never late to delete a Cygwinspecific patch on another ticket.
comment:14 followup: ↓ 15 Changed 15 months ago by
 Status changed from positive_review to needs_info
It would be nice for me to be able to see exactly what changed in the new giac version specifically to fix this problem, so that I can confirm that the problem as I understood it does appear fixed. Unfortunately that's difficult without a publicly viewable change history...
comment:15 in reply to: ↑ 14 Changed 15 months ago by
 Cc parisse added
Replying to embray:
It would be nice for me to be able to see exactly what changed in the new giac version specifically to fix this problem, so that I can confirm that the problem as I understood it does appear fixed. Unfortunately that's difficult without a publicly viewable change history...
The history is embedded in geogebra SVN, cf e.g. history/trac
Perhaps Bernard can point at the right ticket on geogebra's trac.
comment:16 followup: ↓ 18 Changed 15 months ago by
This change is not in SVN geogebra, because it's UI code (Xcas code), not Giac code. I have added if (status!=1) in the nonFLTK code, like in the FLTK code
#ifdef HAVE_LIBFLTK if (status!=1) xcas::Xcas_debugguer(status,contextptr); #else // FIXME Debugguer without FLTK if (status!=1) giac::thread_eval_status(1,contextptr); #endif
It would be a good idea to check the resulting binary anyway. BTW, I will probably update giac to 1.5.063 next week, with a small header fix reported by dimpase.
comment:17 Changed 15 months ago by
 Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.
comment:18 in reply to: ↑ 16 Changed 15 months ago by
Replying to parisse:
This change is not in SVN geogebra, because it's UI code (Xcas code), not Giac code.
Is there a VCS repo somewhere with these Xcas changes? Without such a repo it's quite hard to follow up the changes.
comment:19 followup: ↓ 20 Changed 15 months ago by
No, because Xcas GUI code is not used by other projects. Geogebra has it's own version of a very simple commandline interface (for testing purpose), maybe it would be sufficient for sage as well or any adaptation: https://dev.geogebra.org/trac/browser/trunk/geogebra/giac/src/minigiac/cpp
comment:20 in reply to: ↑ 19 ; followup: ↓ 21 Changed 15 months ago by
Replying to parisse:
No, because Xcas GUI code is not used by other projects.
The current thread is about an issue in Xcas that affects Sagemath. It is a proof that Xcas is used in other projects. If Xcas was on a public VCS then it's very likely that one could merely look up the relevant change there, instead of having this discussion.
comment:21 in reply to: ↑ 20 Changed 15 months ago by
Replying to dimpase:
Replying to parisse:
No, because Xcas GUI code is not used by other projects.
The current thread is about an issue in Xcas that affects Sagemath. It is a proof that Xcas is used in other projects.
I think Bernard's point here (which I agree with, if I understand correctly) is that the original bug I found was caused by multithreaded evaluation code that is needed by Xcas (so that giac commands can be evaluated in a thread without blocking the UI), but that is not needed by nonGUI interfaces.
The problem is that the threaded eval code had a bug, but it also wasn't even needed in the context where Sage was using it, so I just disabled use of the threadedeval in the case where it was affecting me.
The code in comment:16 does appear to be addressing the bug I described in https://trac.sagemath.org/ticket/27385#comment:14
(I still think the code should be using proper condition primitives but that's a bigger change; the above fix will definitely reduce the likelihood of a problem here).
comment:22 Changed 15 months ago by
Yes, I believe it would be simpler and more robust if Sage interface was with a Sage specific giac commandline program (like the one Geogebra is using for regression testing), and not with Xcas's icas that is designed and tested to run with FLTK.
comment:23 Changed 15 months ago by
1.5.063 has been out for a few days and has the header fix.
comment:24 Changed 15 months ago by
 Commit changed from 0be07f536d21b0c1cc2cabb72ce9fab398a8a6ee to 132f5606bae42317f6643737f47700e4031f22d5
Branch pushed to git repo; I updated commit sha1. New commits:
132f560  update giac to 1.5.063

comment:25 Changed 15 months ago by
 Description modified (diff)
 Status changed from needs_info to needs_review
comment:26 Changed 15 months ago by
 Summary changed from update giac to 1.5.061 to update giac to 1.5.063
comment:27 Changed 15 months ago by
 Status changed from needs_review to positive_review
Completely happy with this.
comment:28 Changed 15 months ago by
 Branch changed from u/dimpase/packages/giac15061 to 132f5606bae42317f6643737f47700e4031f22d5
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
update giac to version 1.5.061