danaj / Math-Prime-Util-GMP

Perl prime number module using XS/GMP
Other
16 stars 9 forks source link

trial_factor() disagrees with docs #31

Open hvds opened 2 years ago

hvds commented 2 years ago

The docs say it will always return either 1 or 2 numbers:

  The resulting array will contain either two factors (it succeeded) or the
  original number (no factor was found).
[...]
  Factoring will stop when the input is a prime, one factor
  is found, or the input has been tested for divisibility with all primes less
  than or equal to the limit.

What it actually returns is a list of any factors of 2, 3 and 5 plus the first trial factor found beyond those, plus any residue; this behaviour is also tested in t/15-probprime.t:

% /opt/maths-GMP/bin/perl -MMath::Prime::Util::GMP=trial_factor -wle
  'print join(" * ", trial_factor($_, 100)) for map $_ ** 3, (2, 3, 5, 7, 11, 14, 101)'
2 * 2 * 2
3 * 3 * 3
5 * 5 * 5
7 * 49
11 * 121
2 * 2 * 2 * 7 * 49
1030301
% 

This doesn't seem like particularly coherent behaviour. I think it should either honour the docs as they stand, or perhaps return all factors up to the trial limit. But whatever the behaviour, the docs should match.

I didn't see any difference in docs or behaviour between the release version and github master.

danaj commented 1 year ago

Excellent find. All of the _factor routines were initially put in for testing more than anything else, so should be given more thought to.

The other routines typically find a single factor, and removing the trivial (2,3,5) factors first seemed like a good idea. But I see how that can make using it awkward.

Trial factoring is kind of a special case even internally, as we usually want to do trial factoring until we find something, process that, then possibly continue. Currently it uses from,to arguments to allow this, but arguably using context or an iterator would be better.

danaj commented 1 year ago

I'm rewriting the trial factoring code, and will likely choose to have it return all factors up to the given limit.

It's possible that we might want something like trial_factor_one or something that gives exactly one factor, or something like fortrialfactors { ... } $n,$limit that returns them one by one, and allows early exit with lastfor.