#28101 closed defect (fixed)

update giac to 1.5.0-63

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.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 dimpase)

update giac to the latest version - probably related to the problem on #27385

from the source at www-fourier.ujf-grenoble.fr/~parisse/debian/dists/stable/main/source/giac_1.5.0-63.*

produced tarball: http://users.ox.ac.uk/~coml0531/sage/giac-1.5.0.63.tar.bz2

Change History (28)

comment:1 Changed 15 months ago by dimpase

  • Description modified (diff)

comment:2 Changed 15 months ago by dimpase

  • Description modified (diff)

comment:3 Changed 15 months ago by dimpase

  • Authors set to Dima Pasechnik
  • Branch set to u/dimpase/packages/giac15061
  • Commit set to 277e1d9c0bfda86ed92be31d5f9641a420825475

New commits:

277e1d9update giac to version 1.5.0-61

comment:4 Changed 15 months ago by dimpase

  • Cc embray fbissey added
  • Status changed from new to needs_review

comment:5 Changed 15 months ago by fbissey

  • 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/sage-on-gentoo/blob/master/sci-mathematics/giac/files/giac-1.5.0.61-tex_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/site-packages/sage/libs/giac.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/libs/giac.py", line 209, in sage.libs.giac.?
Failed example:
    B1 = gb_giac(I.gens(),1e-16) # optional - giacpy_sage, long time (1s)
Expected:
    Running a probabilistic check for the reconstructed Groebner basis.
    If successfull, error probability is less than 1e-16 ...
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 1e-16 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.

Last edited 15 months ago by fbissey (previous) (diff)

comment:6 Changed 15 months ago by git

  • Commit changed from 277e1d9c0bfda86ed92be31d5f9641a420825475 to 1fda15e3882261d366bcc8eb2940130c77727962

Branch pushed to git repo; I updated commit sha1. New commits:

1fda15euse "tex.h" rather than <tex.h> in #include

comment:7 Changed 15 months ago by dimpase

  • 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 fbissey

  • Status changed from needs_review to needs_work

What about the giacpy_sage doctest?

comment:9 Changed 15 months ago by git

  • Commit changed from 1fda15e3882261d366bcc8eb2940130c77727962 to 0be07f536d21b0c1cc2cabb72ce9fab398a8a6ee

Branch pushed to git repo; I updated commit sha1. New commits:

0be07f5more verbosity in a long optional test

comment:10 Changed 15 months ago by dimpase

  • 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 fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

LGTM now.

comment:12 Changed 15 months ago by gh-mwageringel

  • 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 cygwin-icas.patch should be deleted, should it not?

comment:13 Changed 15 months ago by dimpase

  • 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 Cygwin-specific patch on another ticket.

comment:14 follow-up: Changed 15 months ago by embray

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

Last edited 15 months ago by embray (previous) (diff)

comment:15 in reply to: ↑ 14 Changed 15 months ago by dimpase

  • 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 follow-up: Changed 15 months ago by parisse

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 non-FLTK 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.0-63 next week, with a small header fix reported by dimpase.

Last edited 15 months ago by embray (previous) (diff)

comment:17 Changed 15 months ago by dimpase

  • 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 dimpase

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 follow-up: Changed 15 months ago by parisse

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 ; follow-up: Changed 15 months ago by 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. 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 embray

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 multi-threaded 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 non-GUI 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 threaded-eval 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).

Last edited 15 months ago by embray (previous) (diff)

comment:22 Changed 15 months ago by parisse

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 fbissey

1.5.0-63 has been out for a few days and has the header fix.

comment:24 Changed 15 months ago by git

  • Commit changed from 0be07f536d21b0c1cc2cabb72ce9fab398a8a6ee to 132f5606bae42317f6643737f47700e4031f22d5

Branch pushed to git repo; I updated commit sha1. New commits:

132f560update giac to 1.5.0-63

comment:25 Changed 15 months ago by dimpase

  • Description modified (diff)
  • Status changed from needs_info to needs_review

comment:26 Changed 15 months ago by dimpase

  • Summary changed from update giac to 1.5.0-61 to update giac to 1.5.0-63

comment:27 Changed 15 months ago by fbissey

  • Status changed from needs_review to positive_review

Completely happy with this.

comment:28 Changed 15 months ago by vbraun

  • Branch changed from u/dimpase/packages/giac15061 to 132f5606bae42317f6643737f47700e4031f22d5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.