correction to RealRoots:-realRootIsolation

[cel34-bath]

Fixed oversight in realRootIsolation method that fails to perform bisection if the polynomial's leadCoefficient is negative.

Lagrange bound was incorrectly implemented, failing to return a positive values whenever the lead coefficient was negative, and included the leading coefficient. This would give a larger lower bound than its upper bound, giving an interval of negative size, causing it to skip the entire bisection process.

This should now be correct (I have tested this but would appreciate another pair of eyes on this just in case).

As mentions in my issue ticket:

d0:=-2*(x1*(x1-1));
A0 := QQ(monoid[x1]);
h0:=sub(d0, A0);
d1:=-d0;
h1:=sub(d1, A0);

realRootIsolation(h0,1)
realRootIsolation(h1,1)

realRootIsolation(h1,1) correctly returns the root intervals as {{−1,0},{0,1}} but realRootIsolation(h0,1) instead returns {{2,−2}}, giving the wrong number of intervals and a lower bound larger than the upper bound.

The associated issue ticket can be found here: #3101

[d-torrance]

This change broke one of the existing tests:

i2 : check(5, "RealRoots", Verbose => true)
 -- capturing check(5, "RealRoots")              -- capture failed; retrying ...
 -- running   check(5, "RealRoots")             
 ulimit -c unlimited; ulimit -t 700; ulimit -m 850000; ulimit -s 8192; ulimit -n 512;  cd /tmp/M2-898930-0/3-rundir/; GC_MAXIMUM_HEAP_SIZE=400M "/usr/bin/M2-binary" --no-randomize --no-readline --silent --stop --print-width 77 --srcdir "/home/profzoom/src/macaulay2/M2/M2" -e 'needsPackage("RealRoots",Reload=>true,FileName=>"/home/profzoom/src/macaulay2/M2/M2/Macaulay2/packages/RealRoots.m2")' <"/tmp/M2-898930-0/2.m2" >>"/tmp/M2-898930-0/2.tmp" 2>&1
/tmp/M2-898930-0/2.tmp:0:1:(3):[12]: (output file) error: Macaulay2 exited with status code 1
/tmp/M2-898930-0/2.m2:0:1: (input file)
M2: *** Error 1
 -- 0.99306 seconds elapsed
../packages/RealRoots.m2:1033:1-1038:1 error:
 -- -- -*- M2-comint -*- hash: 1082028653
 -- 
 -- i1 :  -- /home/profzoom/src/macaulay2/M2/M2/Macaulay2/packages/RealRoots.m2:1038:1: location of test code
 --      
 --          R = QQ[t];
 -- 
 -- i2 :     f = (t-1)^2*(t+3)*(t+5)*(t-6);
 -- 
 -- i3 :     assert(realRootIsolation(f,1/2) == {{-161/32, -299/64}, {-207/64, -23/8}, {23/32, 69/64}, {23/4, 391/64}});
 -- stdio:5:5:(3): error: assertion failed
 -- 
stdio:2:1:(3): error: test(s) #5 of package RealRoots failed.

Here's the new behavior:

i4 : realRootIsolation(f, 1/2)

         1281    2379      1647    183    183  549    183  3111
o4 = {{- ----, - ----}, {- ----, - ---}, {---, ---}, {---, ----}}
          256     512       512     64    256  512     32   512

o4 : List

Since the output of realRootIsolation isn't uniquely determined (I think any pair of rational numbers no more than 1/2 apart from one another that contains the roots of -5, -3, 1, and 6, should work, right?), then maybe the test should be more general. Maybe something like the following?

scan(realRootIsolation(f, 1/2), {-5, -3, 1, 6}, (I, x) -> (
      assert (last I - first I < 1/2);
      assert (x > first I);
      assert (x < last I)))

[cel34-bath]

Thank Doug, I completely forgot to update the test! I will have a proper look in the next few days if needed :) Regards, Corin

…

________________________________ From: Doug Torrance ***@***.***> Sent: Saturday, February 3, 2024 1:32:09 PM To: Macaulay2/M2 ***@***.***> Cc: Corin Lee ***@***.***>; Author ***@***.***> Subject: Re: [Macaulay2/M2] correction to RealRoots:-realRootIsolation (PR #3100) You don't often get email from ***@***.*** Learn why this is important<https://aka.ms/LearnAboutSenderIdentification> CAUTION: This email came from outside of the University. To keep your account safe, only click on links and open attachments if you know the person who sent the email, or you expected to receive this communication. This change broke one of the existing tests: i2 : check(5, "RealRoots", Verbose => true) -- capturing check(5, "RealRoots") -- capture failed; retrying ... -- running check(5, "RealRoots") ulimit -c unlimited; ulimit -t 700; ulimit -m 850000; ulimit -s 8192; ulimit -n 512; cd /tmp/M2-898930-0/3-rundir/; GC_MAXIMUM_HEAP_SIZE=400M "/usr/bin/M2-binary" --no-randomize --no-readline --silent --stop --print-width 77 --srcdir "/home/profzoom/src/macaulay2/M2/M2" -e 'needsPackage("RealRoots",Reload=>true,FileName=>"/home/profzoom/src/macaulay2/M2/M2/Macaulay2/packages/RealRoots.m2")' <"/tmp/M2-898930-0/2.m2" >>"/tmp/M2-898930-0/2.tmp" 2>&1 /tmp/M2-898930-0/2.tmp:0:1:(3):[12]: (output file) error: Macaulay2 exited with status code 1 /tmp/M2-898930-0/2.m2:0:1: (input file) M2: *** Error 1 -- 0.99306 seconds elapsed ../packages/RealRoots.m2:1033:1-1038:1 error: -- -- -*- M2-comint -*- hash: 1082028653 -- -- i1 : -- /home/profzoom/src/macaulay2/M2/M2/Macaulay2/packages/RealRoots.m2:1038:1: location of test code -- -- R = QQ[t]; -- -- i2 : f = (t-1)^2*(t+3)*(t+5)*(t-6); -- -- i3 : assert(realRootIsolation(f,1/2) == {{-161/32, -299/64}, {-207/64, -23/8}, {23/32, 69/64}, {23/4, 391/64}}); -- stdio:5:5:(3): error: assertion failed -- stdio:2:1:(3): error: test(s) #5 of package RealRoots failed. Here's the new behavior: i4 : realRootIsolation(f, 1/2) 1281 2379 1647 183 183 549 183 3111 o4 = {{- ----, - ----}, {- ----, - ---}, {---, ---}, {---, ----}} 256 512 512 64 256 512 32 512 o4 : List Since the output of realRootIsolation isn't uniquely determined (I think any pair of rational numbers no more than 1/2 apart from one another that contains the roots of -5, -3, 1, and 6, should work, right?), then maybe the test should be more general. Maybe something like the following? scan(realRootIsolation(f, 1/2), {-5, -3, 1, 6}, (I, x) -> ( assert (last I - first I < 1/2); assert (x > first I); assert (x < last I))) — Reply to this email directly, view it on GitHub<#3100 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AV3NLP6TNNWOWCZKTSD7243YRY35TAVCNFSM6AAAAABCXMD2OCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVGMZDEOJQGM>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[DanGrayson]

Could you fix the error?

[cel34-bath]

I've further improved/corrected the initial bound (using the Cauchy-Langrange bound) and also dealt with cases with uneven isolating intervals, e.g. if two roots are very close, it would keep bisecting all roots until those roots were isolated and would stop after those roots were isolated. This would mean every 'non-close' root would have a much smaller isolating interval than needed and the output list might not be arranged smallest to largest. Both of these are now fixed.

The test was updated with the value reflecting this change (as mentioned, the intervals only look 'less nice' because the initial bounds previously happened to be divisible by 8, reducing the denominators by that factor)

I'm not sure if the package preamble needs updating to reflect the changes, i.e. version, date, authors(?), but let me know if they do.