Opened 9 years ago
Closed 6 years ago
#10983 closed enhancement (fixed)
new doctest for french book about Sage
Reported by: | casamayou | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-5.13 |
Component: | doctest coverage | Keywords: | |
Cc: | zimmerma, saliola | Merged in: | sage-5.13.beta3 |
Authors: | Alexandre Casamayou, Marc Mezzarobba, Paul Zimmermann | Reviewers: | Dmitrii Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The attached patch includes a new doctest for a book (in french) some people are writing about Sage, under the supervision of Paul Zimmermann.
This patch contains only the doctests for two chapters (about calculus and graphics and solutions for corresponding exercices).
This doctest was tested successfully with Sage 4.6.2 under Debian Squeeze.
Attachments (20)
Change History (59)
comment:1 Changed 9 years ago by
- Cc zimmerma added
Changed 8 years ago by
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Cc saliola added
- Status changed from new to needs_review
comment:4 follow-up: ↓ 5 Changed 8 years ago by
- Status changed from needs_review to needs_info
Hello Paul. I think it is a great idea to include these tests in Sage. However, the files are just python files that contain docstrings; I suspect that one would prefer a patch instead (perhaps I am wrong about this). In order to produce a patch, one needs to first find the best place to put these tests within the sage library. Is there a standard location for such tests?
comment:5 in reply to: ↑ 4 Changed 8 years ago by
Replying to saliola:
Hello Paul. I think it is a great idea to include these tests in Sage. However, the files are just python files that contain docstrings; I suspect that one would prefer a patch instead (perhaps I am wrong about this). In order to produce a patch, one needs to first find the best place to put these tests within the sage library. Is there a standard location for such tests?
Franco, you are right. The standard location is tests/french_book
where there is already
the Python file corresponding to #9395.
Alexandre, please could you make a real patch? Do you know how to do that?
Paul
Changed 8 years ago by
Changed 8 years ago by
comment:6 Changed 8 years ago by
Paul, Here is the log-message for the patch.
Alexandre.
comment:7 Changed 8 years ago by
- Status changed from needs_info to needs_review
comment:8 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:9 Changed 8 years ago by
- Description modified (diff)
- Reviewers set to Dmitrii Pasechnik
comment:10 follow-up: ↓ 11 Changed 8 years ago by
I am slightly worried about timings. Did you check how long the tests take? You might need to add #long time
in various places.
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 8 years ago by
Replying to jdemeyer:
I am slightly worried about timings. Did you check how long the tests take? You might need to add
#long time
in various places.
the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 8 years ago by
Replying to dimpase:
the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?
Guidelines say: anything more than 1 second (on sage.math) should be marked as long time. No test should take more than 30 seconds (even though we have several tests taking more than 30 seconds).
comment:13 in reply to: ↑ 12 ; follow-up: ↓ 14 Changed 8 years ago by
- Status changed from positive_review to needs_work
Replying to jdemeyer:
Replying to dimpase:
the longest test took about 2 minutes on 1.8Gz Intel. Is it something that should be marked as long? Do we have any specific guidelines on this?
Guidelines say: anything more than 1 second (on sage.math) should be marked as long time. No test should take more than 30 seconds (even though we have several tests taking more than 30 seconds).
Oops, sorry. I'll revert to needs_work then! As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.
comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 8 years ago by
Replying to dimpase:
As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.
By breaking tests into several subtests, did you think test as file or as code line ? Shall I break the files 'graphique*.py' and 'sol_graphiques*.py' into shortest files ?
comment:15 in reply to: ↑ 14 ; follow-up: ↓ 16 Changed 8 years ago by
Replying to casamayou:
Replying to dimpase:
As well, there are tests that ought to be broken into several subtests - as they are really long sequences of independent tests. Would be great if this it taken care of, too.
By breaking tests into several subtests, did you think test as file or as code line ? Shall I break the files 'graphique*.py' and 'sol_graphiques*.py' into shortest files ?
Yes, this looks like a good idea . Perhaps there is also a way to have several independent tests in one file, I don't know.
comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 8 years ago by
Replying to dimpase:
Perhaps there is also a way to have several independent tests in one file, I don't know.
Of course you can have several independent tests in one file, just like you can have more than one function or more than one class in a file. Just put several test blocks in the file:
r""" test1 """ r""" test2 """
instead of
r""" test1 test2 """
comment:17 in reply to: ↑ 16 Changed 8 years ago by
Replying to jdemeyer:
Replying to dimpase:
Perhaps there is also a way to have several independent tests in one file, I don't know.
Of course you can have several independent tests in one file, just like you can have more than one function or more than one class in a file. Just put several test blocks in the file:
Attached you will find the test files corrected.
The too long tests were commented: # long time
Does that sound correct ?
Here are the execution time :
$ for i in *.py ; do sage -t $i; done; sage -t "calculus_fixed.py" [11.2 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 11.2 seconds sage -t "graphique_1_fixed.py" [14.9 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 15.0 seconds sage -t "graphique_2_fixed.py" [19.4 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 19.4 seconds sage -t "graphique_3_fixed.py" [8.3 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 8.4 seconds sage -t "sol_calculus_fixed.py" [8.9 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 9.0 seconds sage -t "sol_graphiques_1_fixed.py" [13.0 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 13.1 seconds sage -t "sol_graphiques_2_fixed.py" [4.8 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 4.8 seconds
And with the long tests :
$ for i in *.py ; do sage -t -long $i; done; sage -t -long "calculus_fixed.py" [11.2 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 11.3 seconds sage -t -long "graphique_1_fixed.py" [323.1 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 323.1 seconds sage -t -long "graphique_2_fixed.py" [20.3 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 20.3 seconds sage -t -long "graphique_3_fixed.py" [8.1 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 8.1 seconds sage -t -long "sol_calculus_fixed.py" [8.9 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 9.0 seconds sage -t -long "sol_graphiques_1_fixed.py" [47.3 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 47.3 seconds sage -t -long "sol_graphiques_2_fixed.py" [64.9 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 65.0 seconds
It was the execution of the following instruction that is very long :
sage: f = lambda x, y : (abs(cos((x + I * y) ** 4)) - 1) sage: g2 = implicit_plot(f, (-3, 3), (-3, 3), plot_points=400) # long time
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
comment:18 Changed 8 years ago by
The files *_fixed.py are outdated, because they were tested under sage 7.4.1.
The following files have been updated and tested under sage 7.4.2 and are the only ones to consider:
calculus.py graphique_1.py graphique_2.py graphique_3.py sol_calculus.py sol_graphiques_1.py sol_graphiques_2.py
comment:19 follow-up: ↓ 20 Changed 8 years ago by
One more question.
Should I create a new file trac_10983.patch with a "hg commit" ?
(Sorry... I'm a newbie)
comment:20 in reply to: ↑ 19 Changed 8 years ago by
Replying to casamayou:
One more question.
Should I create a new file trac_10983.patch with a "hg commit" ?
(Sorry... I'm a newbie)
hg commit is what you might sometimes do after applying a patch. It certainly does not create patches. You can create patches using hg, e.g. by hg import. But you don't have to. I'll look at your updates later today.
comment:21 Changed 7 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
Here are updated tests that correspond to the current version of the book. (The division of chapters changed a little bit, but the tests I'm including here roughly correspond to those posted by Alexandre.)
On my machine, using Sage-5.9β0:
Running doctests with ID 2013-03-22-21-36-07-c7afe8cf. Doctesting 3 files. sage -t calculus_doctest.py [246 tests, 6.9 s] sage -t graphique_doctest.py [167 tests, 26.8 s] sage -t premierspas_doctest.py [49 tests, 1.5 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 35.4 seconds cpu time: 30.6 seconds cumulative wall time: 35.2 seconds
comment:22 Changed 6 years ago by
- Status changed from needs_review to needs_work
Most of the doctests break on sage 5.1.beta3. Most output are ordered differently, so there's nothing to be worried about either.
---------------------------------------------------------------------- sage -t --long calculus_doctest.py # 12 doctests failed sage -t --long graphique_doctest.py # 1 doctest failed ----------------------------------------------------------------------
Nathann
Apply trac_10983_doctests_from_french_book.patch
comment:23 Changed 6 years ago by
Ahem. 5.11.beta3 :-P
Nathann
comment:24 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:25 Changed 6 years ago by
- Status changed from needs_work to needs_review
I've made a new patch with a different ordering in the outputs, due to a recent change in Sage (I can't find the corresponding ticket). Unfortunately the book output now slightly differs with what we get in Sage. This patch works fine with Sage 5.11.
Paul
comment:26 Changed 6 years ago by
The ticket which changed the ordering is #9880.
comment:27 Changed 6 years ago by
- Status changed from needs_review to needs_work
indentation should be 4 spaces rather than 2
comment:28 Changed 6 years ago by
for patchbot:
apply trac_10983_doctests_from_french_book.patch
comment:29 Changed 6 years ago by
apply trac_10983_doctests_from_french_book.patch
Changed 6 years ago by
comment:30 follow-up: ↓ 32 Changed 6 years ago by
I have made a new patch with correct indentations.
Unfortunately, there are now 2 doctest errors with simplify_full !!!
for the patchbots:
apply trac_10983_doctests_from_french_book_v2.patch
comment:31 Changed 6 years ago by
- Description modified (diff)
comment:32 in reply to: ↑ 30 Changed 6 years ago by
Replying to chapoton:
I have made a new patch with correct indentations.
Unfortunately, there are now 2 doctest errors with simplify_full !!!
How about the following changes: (I suppose this also means minor corrections to the main text, which I can't do)
diff --git a/sage/tests/french_book/calculus_doctest.py b/sage/tests/french_book/calculus_doctest.py --- a/sage/tests/french_book/calculus_doctest.py +++ b/sage/tests/french_book/calculus_doctest.py @@ -643,7 +643,7 @@ sage: assume(8*n+1>0) sage: u(n) = integrate((4*sqrt(2)-8*t^3-4*sqrt(2)*t^4\ ....: -8*t^5) * t^(8*n), t, 0, 1/sqrt(2)) - sage: (u(n)-v(n)).simplify_full() + sage: (u(n)-v(n)).simplify_exp() 0 Sage example in ./sol/calculus.tex, line 258:: @@ -652,8 +652,8 @@ sage: J = integrate((4*sqrt(2)-8*t^3 \ ....: - 4*sqrt(2)*t^4-8*t^5)\ ....: / (1-t^8), t, 0, 1/sqrt(2)) - sage: J.simplify_full() - pi + 2*log(sqrt(2) + 1) + 2*log(sqrt(2) - 1) + sage: J.simplify_log().simplify_full() + pi Sage example in ./sol/calculus.tex, line 272::
comment:33 follow-up: ↓ 34 Changed 6 years ago by
the second doctest failure looks like a regression in Sage, since the expression 2*e
is not simplified similarly to e
:
sage: e=log(1/2*sqrt(2)*(sqrt(2) + 1)) + log(1/2*sqrt(2)*(sqrt(2) - 1)) sage: e.simplify_full() -log(2) sage: (2*e).simplify_full() 2*log(1/2*sqrt(2)*(sqrt(2) + 1)) + 2*log(1/2*sqrt(2)*(sqrt(2) - 1))
Should I open a ticket for that?
Paul
comment:34 in reply to: ↑ 33 Changed 6 years ago by
Replying to zimmerma:
the second doctest failure looks like a regression in Sage, since the expression
2*e
is not simplified similarly toe
:sage: (2*e).simplify_full() 2*log(1/2*sqrt(2)*(sqrt(2) + 1)) + 2*log(1/2*sqrt(2)*(sqrt(2) - 1))Should I open a ticket for that?
Just as in the patch proposed, you may chain simplifies, getting
sage: (2*e).simplify_log().simplify_full() -log(4)
Not sure it's a Maxima regression, or a proper Sage regression. Opening a trac ticket won't hurt, I suppose.
comment:35 Changed 6 years ago by
comment:36 Changed 6 years ago by
Changed 6 years ago by
comment:37 Changed 6 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
with the new patch, the doctests pass again. Ready for review.
Paul
comment:38 Changed 6 years ago by
- Status changed from needs_review to positive_review
looks good to me.
comment:39 Changed 6 years ago by
- Merged in set to sage-5.13.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
Franco, please could you review this ticket for inclusion in the Sage doctests? This will ensure stability of the example of our book (see #9395).
Thank you, Paul