Implementation of "IsPrime", "Factors", "NextPr", "PrevPr"

[jcbenoist]

Implementation of "IsPrime", "Factors", "NextPr", "PrevPr". Tested on sim. I admit the core code is mostly generated by Claude Opus 4.6. I've done the integration to db48. If this PR is accepted, I'll do a test and will update the help files. I want to know first if this code have a chance to be integrated.

Hold on.. I'll develop tests right now, I have some suspiscions. Meanwhile, you can do comments or observations.

No, all seems OK. I'll publish tests soon.

[jcbenoist]

Tests OK (thanks Claude again). Next step is to add range check in functions. M127 seems a limit for available memory. And test that remultiplying factors gives original number.

I have to know, too, how to check if this is db48x ou db50x, or to check available memory; to set different limits for each hardware?

With reflexion, instead of a range check, is there a graceful detection of stack or RAM overflow to abort a command ? Raw implementation => SEGV in SIM (stack overflow)

[jcbenoist]

I have enhanced the tests. M73 & M83 fails for unclear reasons, since manually testing them in sim is perfectly OK. Maybe for memory or CPU reasons in a test context ? I have good confidence with the algorithms now.

[c3d]

Wow @jcbenoist, this is rather ambitious and looks both pretty useful and pretty good. I'll review in depth, but of course this has good chances of being accepted.

[c3d]

Thanks a lot @jcbenoist.

I will need to do some reformatting of the commit message (to avoid parler en français in the middle of un bout de code qui est principalement in English).

[c3d]

With reflexion, instead of a range check, is there a graceful detection of stack or RAM overflow to abort a command ? Raw implementation => SEGV in SIM (stack overflow)

Yes, but this requires a bit of additional effort. Essentially, you need to check at every step, typically return null on failure, and have most of the code be null-safe. This is how things are done today.

[c3d]

I will make these static members of the bignum class instead of using a bn_ prefix, and I will move these functions to bignum.h.

[c3d]

Note that gcd is already implemented in fraction.cc, but needs to be be moved to bignum.h as well.

[jcbenoist]

I'll do it

[jcbenoist]

done

[jcbenoist]

With reflexion, instead of a range check, is there a graceful detection of stack or RAM overflow to abort a command ? Raw implementation => SEGV in SIM (stack overflow)

Yes, but this requires a bit of additional effort. Essentially, you need to check at every step, typically return null on failure, and have most of the code be null-safe. This is how things are done today.

@c3d Is it rt.available() to check free memory ? Is this sufficient ? What is a typical free memory margin to allow safe operation ? (4 Kbytes ?)

I have programmed rt.available() < 4Kbytes => rt.out_of_memory_error() at strategic places, but now M63+ tests fails, with "out of memory", despite they pass very well without. Is this test pertinent ?

Note : pass with 512 bytes margin. Hope this is safe and memory is well tested this way.

[jcbenoist]

@c3d I wonder if this is better to test rt.available(MIN_MEM) < MIN_MEM rather that rt.available() < MIN_MEM. Calling gc at each iteration risk to slow down the algorithm, or is this negligeable ?

[jcbenoist]

@c3d Do you wish I resolve the conflicts, or do you want to resolve them yourself ?

[c3d]

Sorry @jcbenoist, I thought that you had seen that I had already done most of the work on dev.

The problem with your force-push now is that it conflicts with a number of unrelated changes. So I'm manually lifting:

• Your documentation changes
• What I understand changed in the loop management

[c3d]

For now, since the feature is currently implemented, let me close this, and I will finish the integration work with your most recent branch, as indicated in previous comment. Sorry for the duplicate effort, @jcbenoist, I hope that I will not get it wrong. I was in the middle of cutting out 0.9.16, and your recent changes caught me a bit by surprise.

[c3d]

Hey @jcbenoist, reviewing the out-of-memory code you added, I don't think this is proper, so I will not integrate it.

Consider for example:

@@ -316,6 +319,12 @@ bignum_p pollard_rho_brent(bignum_r n)
 
     for (unsigned c_val = 1; c_val <= 20; c_val++)
     {
+        if (rt.available() < MIN_FACTOR_MEM)
+        {
+            rt.out_of_memory_error();
+            return nullptr;
+        }
+
         bignum_g c = bignum::make(c_val);
         if (!c)
             return nullptr;

The rt.available() call is unnecessary because an rt.out_of_memory_error() will be raised by bignum::make, and we then propagate that by returning nullptr. So there is no need to "preemptively" do that. Furthermore, using rt.available() like this will also cause the code to fail incorrectly with out of memory situations even if the garbage collector would be able to reclaim plenty of memory to fulfil the bignum::make request.

So that change is not necessary and most likely harmful.