Opened 11 years ago
Closed 11 years ago
#10766 closed defect (fixed)
Update ECL to the latest upstream release.
Reported by: | drkirkby | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | packages: standard | Keywords: | |
Cc: | kcrisman, jpflori | Merged in: | sage-4.7.alpha1 |
Authors: | François Bissey | Reviewers: | David Kirkby, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The latest version of the Lisp interpreter ECL, (which is used by Maxima) is 11.1.1, but the version of ECL in Sage is 10.4.1.
Version 10.4.1 is the source of one really major problem on 64-bit OpenSolaris (see #9840), as it creates a broken library libecl.so, which can't be linked to. This stops Sage building as a 64-bit application.
The failure of ECL to build a fully functional 64-bit library is due to non-PIC code being present, which itself was the result of the use of a GCC extension of "computed gotos" in the ECL source code.
http://gcc.gnu.org/onlinedocs/gcc-4.5.2/gcc/Labels-as-Values.html
The updated ECL avoids this GCC extension on Solaris, with a result the library has no text relocation issues.
Apply:
- The new spkg http://spkg-upload.googlecode.com/files/ecl-11.1.1.spkg
- trac_10766-fix_doctest.patch
- trac_10766-fix_symbolic_integration_integral.patch
It is also necessary to update Maxima at the same time. The ticket for Maxima is #10773 which already has positive review.
Attachments (4)
Change History (35)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
Only 4 lines of to comment out for that particular one.
maxima --very-quiet -r "example("arrays");" ;;; Loading #P"/usr/lib64/ecl-11.1.1/sb-bsd-sockets.fas" ;;; Loading #P"/usr/lib64/ecl-11.1.1/sockets.fas" ;;; Loading #P"/usr/lib64/ecl-11.1.1/defsystem.fas" ;;; Loading #P"/usr/lib64/ecl-11.1.1/cmp.fas" a[n]:=n*a[n-1] a := n a n n - 1 a[0]:1 a[5] 120 a[n]:=n a[6] 6 a[4] 24 done
with ecl-10.4.1
maxima --very-quiet -r "example("arrays");" ;;; Loading #P"/Users/frb15/Desktop/Gentoo/usr/lib/ecl-10.4.1/SB-BSD-SOCKETS.fas" ;;; Loading #P"/Users/frb15/Desktop/Gentoo/usr/lib/ecl-10.4.1/SOCKETS.fas" ;;; Loading #P"/Users/frb15/Desktop/Gentoo/usr/lib/ecl-10.4.1/DEFSYSTEM.fas" ;;; Loading #P"/Users/frb15/Desktop/Gentoo/usr/lib/ecl-10.4.1/cmp.fas" ;;; Loading #P"/Users/frb15/Desktop/Gentoo/usr/lib/ecl-10.4.1/sysfun.lsp" a[n]:=n*a[n-1] a := n a n n - 1 a[0]:1 a[5] 120 a[n]:=n a[6] 6 a[4] 24 done
comment:3 Changed 11 years ago by
Since a serious problem has been reported with ECL 11.1.1 with both the version of Maxima currently in Sage (5.21.1) as well as the latest Maxima (5.23.2)
http://www.math.utexas.edu/pipermail/maxima/2011/023868.html
we should probably postpone this until a fix is known.
comment:4 Changed 11 years ago by
If you want to wait rather than fixing maxima that's fine. In the meantime I just finished a patch set. So I am attaching it for future reference.
comment:5 Changed 11 years ago by
There is apparently a fix in ECL's CVS for this.
http://www.mail-archive.com/ecls-list@lists.sourceforge.net/msg00671.html
and was due to a typo by the ECL developer. So it should be possible to upgrade ECL to the latest version after the doctest problems are resolved.
Dave
comment:6 Changed 11 years ago by
- Description modified (diff)
comment:7 Changed 11 years ago by
Corrected patch for white space problems and found I had forgotten one doctest. There is an interesting change in behaviour here. In the old test we have
sage: integrate( ((F(x)-G(x))^2).expand(), x, -infinity, infinity).n() -6.26376265908397e-17 sage: integrate( (F(x)-G(x))^2, x, -infinity, infinity).n() 0
But in the new one the results are not different, expansion or not. So the expression is probably always expanded now.
comment:8 follow-up: ↓ 9 Changed 11 years ago by
Added the patch correcting the problem reported earlier. I got it for the following commit http://ecls.git.sourceforge.net/git/gitweb.cgi?p=ecls/ecl;a=commit;h=ce19c67a1b9f63cd232e7c0a621b6ca87aaa7214
comment:9 in reply to: ↑ 8 Changed 11 years ago by
Replying to fbissey:
Added the patch correcting the problem reported earlier. I got it for the following commit http://ecls.git.sourceforge.net/git/gitweb.cgi?p=ecls/ecl;a=commit;h=ce19c67a1b9f63cd232e7c0a621b6ca87aaa7214
Can you explain how you convert that git patch to one using GNU patch that can be used to update the package - since we have now agreed to make GNU patch a standard package, we should use that to update the file and not use 'cp' as before.
Can you post an spkg for review.
I've created a ticket for the Maxima upgrade (#10773) and have posted an .spkg on that ticket, which now needs review.
Dave
comment:10 Changed 11 years ago by
OK, I will put things in motion to upload an spkg. The patch I posted works fairly well with GNU patch as far as I am concerned. But path may need adjusting to follow standard sage procedure. I'll to look up a spkg that uses patch to make sure.
comment:11 Changed 11 years ago by
The new spkg can be found here: http://spkg-upload.googlecode.com/files/ecl-11.1.1.spkg
Two more things to really think about: what to do about the symbolic/integration/integral.py test and should we add a test to make sure that the problem solved by the included patch is/stay fixed.
Changed 11 years ago by
show changes to SPKG.txt, spkg-install and the patch for reviewer only - do not apply.
comment:12 Changed 11 years ago by
- Cc kcrisman added
comment:13 follow-up: ↓ 14 Changed 11 years ago by
Currently testing the ECL/Maxima update here on OS X 10.4 PPC.
Just a couple questions - most doctests seem to be fixed with ellipses, but a couple not. Any particular reason? Also, have these been checked on sage.math or some other Linux system, or an Intel system? Just curious.
Also, the last patch has the same type of issue as has been raised on sage-devel in the past about 0 versus very small epsilon. I don't know if it's worth bothering about, but anyway in case someone remembers what has been done with such in the past...
comment:14 in reply to: ↑ 13 Changed 11 years ago by
Replying to kcrisman:
Currently testing the ECL/Maxima update here on OS X 10.4 PPC.
Good. Let us know how you got on.
Just a couple questions - most doctests seem to be fixed with ellipses, but a couple not. Any particular reason? Also, have these been checked on sage.math or some other Linux system, or an Intel system? Just curious.
These ellipses really bother me, as very often we permit changes 1000 or more times greater than necessary. If we expect 1.82
, but a system gives 1.81999999999999
then changing the doctest to 1.8...
is just permitting differences far greater than we have observed. That would cause 1.85
to pass, which seems stupid to me, but it seems to be the Sage way of doing things. As such, I think the doctest changes are ok.
Francois checked on an Intel Linux system and got the same results as me on an Intel OpenSolaris system. We have not to my knowledge checked on any other Intel system, such as sage.math.
If anything, I would have expected numerical issues to show up on the SPARC processor, as the floating point processor is very different to AMD/Intel. However, there are no issues on SPARC - I checked on t2.math.
I think this is a case of where the buildbot can be useful - just test on all machines and note any issues.
Also, the last patch has the same type of issue as has been raised on sage-devel in the past about 0 versus very small epsilon. I don't know if it's worth bothering about, but anyway in case someone remembers what has been done with such in the past...
I don't have a strong opinion on this. I think such small numbers can be expected. I think we should report this to the ECL list, as it was the ECL update which caused this, not the Maxima upgrade. But unless the ECL developers acknowledges a bug and fixes it, I would just leave it as it is now.
comment:15 Changed 11 years ago by
- Reviewers set to David Kirkby
- Status changed from new to needs_review
As far as I'm aware, all the issues are resolved, so this is due for review. I'm personally happy with it, but lets see what happens on PPC before giving it a positive review. If PPC passes ok, then I suggest this gets a positive review.
In the event there are issues on one of the buildbot machines, which includes sage.math, we will be made aware of it. There seems no point in testing on lots of machines now, when the buildbot can do it with far less effort.
Dave
comment:16 Changed 11 years ago by
- Cc jpflori added
comment:17 Changed 11 years ago by
- Description modified (diff)
comment:18 Changed 11 years ago by
- Description modified (diff)
comment:19 Changed 11 years ago by
- Reviewers changed from David Kirkby to David Kirkby, Karl-Dieter Crisman
- Status changed from needs_review to positive_review
Yeah, I looked at the few changes and our patch, and all seems in order. I don't have a problem with any of the doctest changes either, but thought I'd ask since opinions may differ. Passes as many relevant tests as I had time to ask it to do today.
comment:20 Changed 11 years ago by
- Milestone changed from sage-4.6.2 to sage-4.7
Changing milestone in the hopes it will get in for 4.7 :)
comment:21 Changed 11 years ago by
Since I have put them I think I should explain my choices of ellipses (or not).
First we are doing numerical computation here, and as far as I can see, not in arbitrary precision. In that context having a lot of significant figures is usually useless. It just test your FPU and your implementation. So for example we now have
1.5430806348152437 instead of
1.543080634815244, big deal, next iteration will probably have something different there, it may very well revert to print one less decimal which is probably the only difference here. If we had printed as many decimals with the old ecl we may have had the same result. By putting ellipses here I am taking away the last few significant figures which are quite possibly arbitrary. I guess I could check with wolfram alpha at an arbitrary precision as well.
If we had something like 1.85 and 1.849999999 that would more problematic. But it is a symptom of the fact that we probably should just test a number of significant figures. Instead of having the raw results print 10 significant figures for example, more if you do arbitrary precision computations.
For this one I put the exact answer returned by maxima:
sage -t -force_lib "devel/sage-main/sage/calculus/calculus.py" ********************************************************************** File "/usr/share/sage/devel/sage-main/sage/calculus/calculus.py", line 677: sage: f.nintegrate(x,0,1) Expected: (-480.00000000000011, 5.3290705182007538e-12, 21, 0) Got: (-480.00000000000006, 5.3290705182007538e-12, 21, 0) **********************************************************************
I did that because of the text that goes with the test:
Despite appearance, `f` is really very close to 0, but one gets a nonzero value since the definition of ``float(f)`` is that it makes all constants inside the expression floats, then evaluates each function and each arithmetic operation using float arithmetic:: sage: float(f) -480.0 Computing to higher precision we see the truth:: sage: f.n(200) -7.4992740280181431112064614366622348652078895136533593355718e-13 sage: f.n(300) -7.49927402801814311120646143662663009137292462589621789352095066181709095575681963967103004e-13 Now numerically integrating, we see why the answer is wrong:: sage: f.nintegrate(x,0,1) (-480.00000000000006, 5.3290705182007538e-12, 21, 0) It is just because every floating point evaluation of return -480.0 in floating point.
I thought it would be best to have the exact value. However anything starting with -480.00 would clearly have been good enough.
The test in symbolic/integration/integral.py is more interesting. It documented a behavior of ecl/maxima when you used ".expand" or not and the influence it had on results, before the patch:
Check if #6189 is fixed (which, by the way, also demonstrates it's not always good to expand):: sage: n = N; n <function numerical_approx at ...> sage: F(x) = 1/sqrt(2*pi*1^2)*exp(-1/(2*1^2)*(x-0)^2) sage: G(x) = 1/sqrt(2*pi*n(1)^2)*exp(-1/(2*n(1)^2)*(x-n(0))^2) sage: integrate( (F(x)-F(x))^2, x, -infinity, infinity).n() 0.000000000000000 sage: integrate( ((F(x)-G(x))^2).expand(), x, -infinity, infinity).n() -6.26376265908397e-17 sage: integrate( (F(x)-G(x))^2, x, -infinity, infinity).n() 0
Now the result is the same whether we expand or not. So the test is
1) moot
2) virtually reopen #6189 since we now always have a non-zero result
I think for this one we need a better example and a way of making this integral behave.
In summary most of the numerical noise observed is due to the fact that ecl-11.1.1 print one more decimal place by default. Because this may change and the level of precision doesn't make any sense for most purpose I decided to put ellipses. The more juicy bits where I didn't do that are open to interesting discussion.
comment:22 Changed 11 years ago by
Note to the release manager
This must be merged with #10773, which updates Maxima. Updating just Maxima or just ECL will cause problems - they must be done together.
Dave
comment:23 follow-up: ↓ 27 Changed 11 years ago by
- Milestone changed from sage-4.7 to sage-4.6.2
To my knowledge 4.6.2 is not in any sort of "feature freeze" yet, with no release candidates released - only alphas. So I've set this back to 4.6.2. I've asked on sage-devel if it can go in 4.6.2, but so far no response.
comment:24 Changed 11 years ago by
I installed the new ECL and Maxima spkgs and related patches and ran "make ptest" (running on Ubuntu 10.10, intel x86_64) and everything was fine.
comment:25 Changed 11 years ago by
- Milestone changed from sage-4.6.2 to sage-4.7
comment:26 Changed 11 years ago by
My bad, I meant everything was ok after successfully building Maxima.
To build Maxima, I need to install texinfo package on my system.
Afterwards everything runs fine.
Sorry for the misunderstanding.
comment:27 in reply to: ↑ 23 ; follow-up: ↓ 28 Changed 11 years ago by
Replying to drkirkby:
To my knowledge 4.6.2 is not in any sort of "feature freeze" yet, with no release candidates released - only alphas.
sage-4.6.2.rc0 is scheduled to be tested any time now. So I would rather not merge this in the 4.6.2 cycle (unless you give me a very good reason).
comment:28 in reply to: ↑ 27 Changed 11 years ago by
Replying to jdemeyer:
Replying to drkirkby:
To my knowledge 4.6.2 is not in any sort of "feature freeze" yet, with no release candidates released - only alphas.
True, but...
sage-4.6.2.rc0 is scheduled to be tested any time now. So I would rather not merge this in the 4.6.2 cycle (unless you give me a very good reason).
Usually spkg upgrades are best for early in the cycle, I agree, especially ones like Maxima that have a fair amount of use. I'm sure this will be first up in 4.7 - assuming we can get the texinfo issue resolved.
comment:29 Changed 11 years ago by
- Component changed from algebra to packages
comment:30 Changed 11 years ago by
- Description modified (diff)
comment:31 Changed 11 years ago by
- Merged in set to sage-4.7.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
List of doctests in need of fixing for this to happen:
formatting and numeric noise
Numeric noise
Numerical and format. The first one should be closely looked at.
Numerical noise.
Numerical noise
Numerical noise. Same as reported by David on sage-devel. It all should be straightforward if tedious.