Opened 6 years ago
Closed 6 years ago
#22178 closed defect (fixed)
Update the Sage Development Process documentation for new users
Reported by: | jessicapalencia | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.6 |
Component: | documentation | Keywords: | doc, documentation, developer's guide, days82 |
Cc: | jen, VivianePons, nthiery, egunawan, jdemeyer, embray, dimpase | Merged in: | |
Authors: | Jessica Striker, Jennifer Balakrishnan, Travis Scrimshaw | Reviewers: | Dima Pasechnik, Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | ffcc00f (Commits, GitHub, GitLab) | Commit: | ffcc00ff45c93f86540c36101a392bc5d0445f7b |
Dependencies: | Stopgaps: |
Description
At Sage Days 82, we referred many new users to the Sage Development Process documentation and found many errors or points that need clarification.
Change History (39)
comment:1 Changed 6 years ago by
- Branch set to u/jessicapalencia/22178SageDevel
- Commit set to 5a9accdee6b391f6129ef2d402df371bf0d4cfeb
comment:2 Changed 6 years ago by
- Status changed from new to needs_review
comment:3 Changed 6 years ago by
- Cc nthiery added
comment:4 Changed 6 years ago by
comment:5 Changed 6 years ago by
- Milestone changed from sage-7.5 to sage-7.6
comment:6 Changed 6 years ago by
- Cc egunawan added
comment:7 follow-up: ↓ 8 Changed 6 years ago by
- Cc jdemeyer embray dimpase added
+ If your system supports multiprocessing and you want to use multiple
+ processors to build Sage, replace the last line above by::
+
+ [user@localhost sage]$ make -jNUM
The current recommendation seems to be to use
export MAKE="make -jNUM" make
instead. See discussion in #21610.
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 6 years ago by
comment:9 in reply to: ↑ 8 ; follow-up: ↓ 11 Changed 6 years ago by
Replying to dimpase:
indeed, as indicated in #21610, one can get away with toplevel
make -jNUM
, but only after some work, and assumingmake
's version is at least 4.2.1.
I run make -j10
all the time on OS X Sierra (make
version 3.81) without doing anything else and have no apparent problems. Does that just mean that the process falls back to a single job for parts of the build, like the documentation?
comment:10 Changed 6 years ago by
On a minor note, there are a couple really long lines added in this commit. Please break them to keep all lines in a file to about the same length. That makes them easier to read.
comment:11 in reply to: ↑ 9 ; follow-up: ↓ 12 Changed 6 years ago by
Replying to paulmasson:
Replying to dimpase:
indeed, as indicated in #21610, one can get away with toplevel
make -jNUM
, but only after some work, and assumingmake
's version is at least 4.2.1.I run
make -j10
all the time on OS X Sierra (make
version 3.81) without doing anything else and have no apparent problems. Does that just mean that the process falls back to a single job for parts of the build, like the documentation?
most probably the latter. Could you try the other way on the documentation, whether there is a speed difference for you?
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 6 years ago by
Replying to dimpase:
Replying to paulmasson:
Replying to dimpase:
indeed, as indicated in #21610, one can get away with toplevel
make -jNUM
, but only after some work, and assumingmake
's version is at least 4.2.1.I run
make -j10
all the time on OS X Sierra (make
version 3.81) without doing anything else and have no apparent problems. Does that just mean that the process falls back to a single job for parts of the build, like the documentation?most probably the latter. Could you try the other way on the documentation, whether there is a speed difference for you?
With plain make -j10
documentation takes 1284 seconds to build, with export
it takes 1155 seconds. Not enough of a difference for me to stop using the simpler command.
comment:13 in reply to: ↑ 12 Changed 6 years ago by
Replying to paulmasson:
Replying to dimpase:
Replying to paulmasson:
Replying to dimpase:
indeed, as indicated in #21610, one can get away with toplevel
make -jNUM
, but only after some work, and assumingmake
's version is at least 4.2.1.I run
make -j10
all the time on OS X Sierra (make
version 3.81) without doing anything else and have no apparent problems. Does that just mean that the process falls back to a single job for parts of the build, like the documentation?most probably the latter. Could you try the other way on the documentation, whether there is a speed difference for you?
With plain
make -j10
documentation takes 1284 seconds to build, withexport
it takes 1155 seconds. Not enough of a difference for me to stop using the simpler command.
My bad, documentation build apparently somehow gets hold of '-j10'---you could see this by looking at 'top' output and observing about 10 python processes running.
What would matter is building of Sage packages - each of them would be built with '-j1'.
comment:14 follow-up: ↓ 17 Changed 6 years ago by
On one machine, when I built with MAKE='make -j6'
, make build
took 99 minutes. With MAKE
unset, make -j6 build
took 107 minutes.
comment:15 Changed 6 years ago by
- Commit changed from 5a9accdee6b391f6129ef2d402df371bf0d4cfeb to 20b18fd88bfb1e4a942056fcaeb58fefb0fabc1c
Branch pushed to git repo; I updated commit sha1. New commits:
20b18fd | fixed long lines and changed make -jNUM
|
comment:16 Changed 6 years ago by
We addressed all the comments above.
comment:17 in reply to: ↑ 14 Changed 6 years ago by
Replying to jhpalmieri:
On one machine, when I built with
MAKE='make -j6'
,make build
took 99 minutes. WithMAKE
unset,make -j6 build
took 107 minutes.
I did make distclean
followed by make -j8
on an 8-core Linux machine
I got, with MAKE
set to make -j8
real 41m35.245s user 259m17.166s sys 13m55.563s
and with MAKE
unset (don't forget to actually run unset MAKE
before trying)
real 55m40.123s user 219m20.552s sys 13m30.681s
which is almost 30% slowdown (w.r.t. the wall clock).
comment:18 Changed 6 years ago by
isn't
$ export MAKE='make -jNUM' $ $MAKE
more effective than
$ MAKE='make -jNUM' make
(unless for some reason you really do not want to keep building with -jNUM
next time?)
comment:19 Changed 6 years ago by
I think the right place to give full details about how to set the MAKE
environment variable is in the "Environment variables" section of the Sage installation guide. So the brief comments in the developer's guide don't need to be perfect, for example with respect to
$ MAKE='make -jNUM' make
vs.
$ export MAKE='make -jNUM' $ make
comment:20 Changed 6 years ago by
- Reviewers set to Dima Pasechnik
- Status changed from needs_review to positive_review
OK. Other reviewers, please feel free to write themselves in.
comment:21 Changed 6 years ago by
- Reviewers changed from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe
comment:22 Changed 6 years ago by
- Status changed from positive_review to needs_work
Two very minor things:
- We are trying to standardize
.. NOTE::
instead of.. note::
. - Some of your line breaks make some lines unnaturally short to me.
The more important of the two is the first one.
comment:23 Changed 6 years ago by
- Status changed from needs_work to positive_review
when the day comes, someone will make a script to fix the other 500+ occurences...
$ grep -R "note::" src/sage/ | wc -l 513
comment:24 Changed 6 years ago by
- Status changed from positive_review to needs_work
travis@apricot:~/sage-build$ grep -R "NOTE::" src/sage/ | wc -l 1636
We don't have to make it worse.
comment:25 Changed 6 years ago by
- Status changed from needs_work to positive_review
Well, instead of arguing here, make a ticket to fix this Sage-wise :-)
With about 25% for note::
vs 75% for NOTE::
, it's a bit premature to make a fuss about it.
comment:26 Changed 6 years ago by
Is there a patchbot script that checks on these things?
comment:27 Changed 6 years ago by
IMHO one wants to propose changes like this, which are more of matter of taste than anything else, it's more appropriate to push a commit to fix it, instead of keep setting this to "needs work". Because the latter isn't really the case.
comment:28 Changed 6 years ago by
- Branch changed from u/jessicapalencia/22178SageDevel to u/jhpalmieri/22178SageDevel
comment:29 Changed 6 years ago by
- Branch changed from u/jhpalmieri/22178SageDevel to u/tscrim/22178SageDevel
- Commit changed from 20b18fd88bfb1e4a942056fcaeb58fefb0fabc1c to e1a96f5d0ea218cd11c769f19bdf380054b6f460
- Status changed from positive_review to needs_review
Instead of sweeping things under the rug, let's make sure things are done right. These are not a "matter of taste" but conventions we have set in Sage that we should adhere to. Here's you're branch, where I noticed some other things that needed fixing too.
New commits:
e1a96f5 | Fixing things that need to be fixed before a positive review.
|
comment:30 Changed 6 years ago by
- Commit changed from e1a96f5d0ea218cd11c769f19bdf380054b6f460 to ffcc00ff45c93f86540c36101a392bc5d0445f7b
comment:31 Changed 6 years ago by
- Status changed from needs_review to positive_review
Note NOTE
vs note
is not a written convention. Anyway.
comment:32 follow-up: ↓ 33 Changed 6 years ago by
comment:33 in reply to: ↑ 32 Changed 6 years ago by
Replying to tscrim:
See http://doc.sagemath.org/html/en/developer/coding_basics.html
A NOTE block for tips/tricks (optional).
Does it say it must be all-caps? No. Actually I don't like all-caps, they scream too much at your face...
comment:34 Changed 6 years ago by
I know this ticket is already on positive review, but I'd like to suggest to change it to
make V=0
This makes the output much less overwhelming.
comment:35 Changed 6 years ago by
Actually I never heard about V=0
. Perhaps it's less common knowledge than you think, and a more detailed explanation ought to be added.
comment:36 Changed 6 years ago by
We already have an installation manual where fine points of make
should go. I would suggest documenating make V=0
there, and perhaps putting a pointer to the installation manual in the developer's manual.
comment:37 Changed 6 years ago by
I added this feature some time last year in #21539. Looks like I forgot to advertise it in the documentation.
comment:38 Changed 6 years ago by
#22227 adds the documentation of make V=0.
comment:39 Changed 6 years ago by
- Branch changed from u/tscrim/22178SageDevel to ffcc00ff45c93f86540c36101a392bc5d0445f7b
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Fixes to Sage Developer's guide documentation