Doctest memory limit

44 views
Skip to first unread message

Michael Orlitzky

unread,
Mar 22, 2021, 8:21:43 PM3/22/21
to sage-devel
Is anyone very much in love with the --memlimit (default: 3300MB)
option to the sage -t command?

Once again it has completely broken testing on some systems. We could
try to guess a new, higher limit... or just admit that maybe it's not a
great idea after all and delete the thing. The latter is what I'm about
to propose on https://trac.sagemath.org/ticket/31395

A few points:

* The failure of the default limit has been and will always be a
recurring problem as sage requires more and more memory. Every
time we hit it, a bunch of machines are broken until the limit
can be raised in the "develop" branch.

* The default memory limit exists for precisely one doctest (which 
I've refactored to still be tested, modulo the next bullet point).

* Testing out-of-memory conditions doesn't test what you'd expect, 
since if you _actually_ run out of memory, all hell breaks loose
on the system at the same time as your graceful error handling 
kicks in.

* A global limit is likely incorrect, as would be revealed if there 
were more than one doctest using it. Each test needs the limit to
be low enough to trigger a failure, but not low enough to crash
the rest of sage. Both of these numbers are test- and system-
dependent.

* Reimplementing "ulimit -v" in a mathematics suite is a waste of
development resources.

I'd rather just delete it and generalize the one existing doctest with
something like a "with memlimit(...)" context manager in the unlikely
event that we ever have another test for OOM behavior.


François Bissey

unread,
Mar 22, 2021, 8:31:45 PM3/22/21
to sage-...@googlegroups.com
It has been a pain for Steve Trogdon in sage-on-gentoo on an ongoing basis
ever since it was included. I have somehow been blessed with enough RAM on
all my dev machines. You sum and articulate my position on the matter quite
nicely and even with additional bullet points (the last three are really the
ones that have been on the back of my mind for awhile).

You could say I am biased but I wouldn’t be sorry to say goodbye to the thing.

François
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/fb887a37abdddeb77e05d433983b3d1ae8dca149.camel%40orlitzky.com.

Volker Braun

unread,
Mar 23, 2021, 8:16:28 PM3/23/21
to sage-devel
I don't understand what the big pain point is, if Steve doesn't like the limit it he can just pick a different value no?

We do need some way of keeping the memory usage in check, people naturally want to show off largeish computations and we run the risk to require a beefy developer machine to be able to develop for Sage.

As long as we have 32-bit buildbots the limit can't be much higher, but the writing is on the wall than 32-bit is going away eventually.

From my point of view its better to have some limit that provides immediate feedback during development than me having to maintain a special buildbot to limit memory usage and police tickets where developers don't have an easy way to reproduce.   

François Bissey

unread,
Mar 23, 2021, 8:27:45 PM3/23/21
to sage-...@googlegroups.com
Running the doctests plong style just regularly fails for him on some machines with
memory errors. Re-running the tests with higher limits usually works. But the default
are obviously too low on some of his machines.

Dima Pasechnik

unread,
Mar 24, 2021, 5:50:31 AM3/24/21
to sage-devel
On Wed, Mar 24, 2021 at 12:16 AM Volker Braun <vbrau...@gmail.com> wrote:
>
> I don't understand what the big pain point is, if Steve doesn't like the limit it he can just pick a different value no?
>
> We do need some way of keeping the memory usage in check, people naturally want to show off largeish computations and we run the risk to require a beefy developer machine to be able to develop for Sage.
>
> As long as we have 32-bit buildbots the limit can't be much higher, but the writing is on the wall than 32-bit is going away eventually.
>
> From my point of view its better to have some limit that provides immediate feedback during development than me having to maintain a special buildbot to limit memory usage and police tickets where developers don't have an easy way to reproduce.

How about having make targets with and without these limits? Then most
people would do

make ptetslonghimem
(or rather ptestlong)

while others would do

make ptestlonglowmen

etc.

(and the default should be 0, not 3300, if you ask me)
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/sage-devel/536abe7e-c9fd-4336-aa03-8d430508a472n%40googlegroups.com.

Dima Pasechnik

unread,
Mar 24, 2021, 5:55:06 AM3/24/21
to sage-devel
On Wed, Mar 24, 2021 at 9:50 AM Dima Pasechnik <dim...@gmail.com> wrote:
>
> On Wed, Mar 24, 2021 at 12:16 AM Volker Braun <vbrau...@gmail.com> wrote:
> >
> > I don't understand what the big pain point is, if Steve doesn't like the limit it he can just pick a different value no?
> >
> > We do need some way of keeping the memory usage in check, people naturally want to show off largeish computations and we run the risk to require a beefy developer machine to be able to develop for Sage.
> >
> > As long as we have 32-bit buildbots the limit can't be much higher, but the writing is on the wall than 32-bit is going away eventually.
> >
> > From my point of view its better to have some limit that provides immediate feedback during development than me having to maintain a special buildbot to limit memory usage and police tickets where developers don't have an easy way to reproduce.
>
> How about having make targets with and without these limits? Then most
> people would do
>
> make ptetslonghimem
> (or rather ptestlong)
>
> while others would do
>
> make ptestlonglowmen

feels like MSDOS to me, though :-)

Michael Orlitzky

unread,
Mar 24, 2021, 6:18:23 AM3/24/21
to sage-...@googlegroups.com
On Tue, 2021-03-23 at 17:16 -0700, Volker Braun wrote:
> I don't understand what the big pain point is, if Steve doesn't like the
> limit it he can just pick a different value no?

You can, but "make ptestlong" no longer works. And the correct number
to use is not obvious.

The "optional - memlimit" tests get run whenever there's any limit
set... not only when there's an _appropriate_ limit set. The one
doctest that needs a memory limit tries to construct a matrix with 2^31
entries in GF(2), so you need to guess a limit that's high enough not
to crash sage (> 3300 MB) but low enough that you can't allocate a that
matrix. Both of those numbers (the lower and upper limits), which are
already pretty close together, change occasionally as libraries and
toolchains do.

>
> We do need some way of keeping the memory usage in check, people naturally
> want to show off largeish computations and we run the risk to require a
> beefy developer machine to be able to develop for Sage.
>

I haven't done the requisite git archaeology, but I don't think this is
the purpose of --memlimit. It's there because there's one doctest for
OOM behavior, and you can't can't count on it running out of memory
without a limit being set.

In the past, whenever we've hit the limit, we've just increased the
limit.

I agree that we should try to keep the test suite's memory usage down,
but I think the CI is a better place to do it:

* the amount of RAM that gets used is dependent upon the system
running the tests

* we can use the standard "ulimit -v" there

* In the CI, we don't have to guess at a limit that's small enough 
to cause the allocation failures that are being doctested, because
we're not trying to cause allocation failures

* breakage will only affect developers and can be quickly fixed 
(ideally before being merged)




Reply all
Reply to author
Forward
0 new messages