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:

Status badges

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 jen

  • Branch set to u/jessicapalencia/22178SageDevel
  • Commit set to 5a9accdee6b391f6129ef2d402df371bf0d4cfeb

New commits:

5a9accdFixes to Sage Developer's guide documentation

comment:2 Changed 6 years ago by jen

  • Status changed from new to needs_review

comment:3 Changed 6 years ago by jen

  • Cc nthiery added

comment:4 Changed 6 years ago by jen

  • Authors set to Jessica Striker, Jennifer Balakrishnan

comment:5 Changed 6 years ago by jen

  • Milestone changed from sage-7.5 to sage-7.6

comment:6 Changed 6 years ago by jen

  • Cc egunawan added

comment:7 follow-up: Changed 6 years ago by mkoeppe

  • 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: Changed 6 years ago by dimpase

Replying to mkoeppe:

The current recommendation seems to be to use

    export MAKE="make -jNUM"
    make

instead. See discussion in #21610.

indeed, as indicated in #21610, one can get away with toplevel make -jNUM, but only after some work, and assuming make's version is at least 4.2.1.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 6 years ago by paulmasson

Replying to dimpase:

indeed, as indicated in #21610, one can get away with toplevel make -jNUM, but only after some work, and assuming make'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 paulmasson

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: Changed 6 years ago by 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 assuming make'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: Changed 6 years ago by 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 assuming make'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 dimpase

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 assuming make'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.

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: Changed 6 years ago by jhpalmieri

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 git

  • Commit changed from 5a9accdee6b391f6129ef2d402df371bf0d4cfeb to 20b18fd88bfb1e4a942056fcaeb58fefb0fabc1c

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

20b18fdfixed long lines and changed make -jNUM

comment:16 Changed 6 years ago by jessicapalencia

We addressed all the comments above.

comment:17 in reply to: ↑ 14 Changed 6 years ago by dimpase

Replying to jhpalmieri:

On one machine, when I built with MAKE='make -j6', make build took 99 minutes. With MAKE 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 dimpase

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 jhpalmieri

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 dimpase

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

  • Reviewers changed from Dima Pasechnik to Dima Pasechnik, Matthias Koeppe

comment:22 Changed 6 years ago by tscrim

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

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

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

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

Is there a patchbot script that checks on these things?

comment:27 Changed 6 years ago by dimpase

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 jhpalmieri

  • Branch changed from u/jessicapalencia/22178SageDevel to u/jhpalmieri/22178SageDevel

comment:29 Changed 6 years ago by tscrim

  • Authors changed from Jessica Striker, Jennifer Balakrishnan to Jessica Striker, Jennifer Balakrishnan, Travis Scrimshaw
  • 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:

e1a96f5Fixing things that need to be fixed before a positive review.

comment:30 Changed 6 years ago by git

  • Commit changed from e1a96f5d0ea218cd11c769f19bdf380054b6f460 to ffcc00ff45c93f86540c36101a392bc5d0445f7b

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

27ab618Trac 22178: change two instances of '.. note' to '.. NOTE'.
ffcc00fMerge branch 'u/jhpalmieri/22178SageDevel' of git://trac.sagemath.org/sage into u/tscrim/22178SageDevel

comment:31 Changed 6 years ago by dimpase

  • Status changed from needs_review to positive_review

Note NOTE vs note is not a written convention. Anyway.

comment:33 in reply to: ↑ 32 Changed 6 years ago by dimpase

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 mkoeppe

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 dimpase

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 jhpalmieri

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 mkoeppe

I added this feature some time last year in #21539. Looks like I forgot to advertise it in the documentation.

Last edited 6 years ago by mkoeppe (previous) (diff)

comment:38 Changed 6 years ago by mkoeppe

#22227 adds the documentation of make V=0.

comment:39 Changed 6 years ago by vbraun

  • Branch changed from u/tscrim/22178SageDevel to ffcc00ff45c93f86540c36101a392bc5d0445f7b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.