Hi Mike,
I've made the suggested change and pushed to GitHub.
Next time the addon is released, the change will be included.

I wonder if it makes sense for suggestions like this to be provided as a
GitHub pull request where possible?
* less chance of miscommunication between report and fix
* easier for maintainers
* links discussion to change

For simple changes like this, that can be done entirely from the GitHub
website.

 * Navigate to:
https://github.com/jsoftware/math_misc/blob/master/pollard.ijs
 * Click the pencil icon on the right ("Edit this file")
 * Make the desired changes
 * Scroll to bottom of page and fill out Commit changes fields
 * Choose option to "Create a *new branch* for this commit and start a pull
request"
 * Click the "Propose changes" button
 * Add commentary/justification to "Open a pull request form"
 * Click the "Create pull request" button


On Fri, Feb 12, 2021 at 5:30 AM 'Michael Day' via Programming <
[email protected]> wrote:

> Further to my delve in the scripts in ~addons/math/misc,  I've just
> noticed a
> misleading comment in pollard.ijs .
>
> The script offers implementations of two Pollard factorisation algorhithms.
>
> The relevant comments are (with a bit of context):
> NB. examples:
> NB.
> NB.    ]x=. (,*/) x: p: 1e7 30101
> NB. 179424691 351599 63085541930909
> NB.    pollardpm1 {: x
> NB. 351599 335
>
> Well - when you actually run it,  this is the result:
>
>        ]x=. (,*/) x: p: 1e7 30101
> 179424691 351599 63085541930909
>        pollardpm1 {: x
> 179424691 2556
>
> Initially,  I thought this might be a bug,   but no.  The result in the
> comment arises from the other function which does pollard rho
> factorisation:
>     pollardrho {: x
> 351599 335
>
> Indeed,  we see that x is defined as the product of the factors found by
> the two methods.
>
> No doubt this little slip has been around for some years.
>
> I suggest the comments be amended to read:
>
> NB. examples:
> NB.
> NB.    ]x=. (,*/) x: p: 1e7 30101
> NB. 179424691 351599 63085541930909
> NB.    pollardpm1 {: x
> NB. 179424691 2556
>     et seq... (no changes needed)
>
> Cheers,
>
> Mike
>
> --
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
>
> ----------------------------------------------------------------------
> For information about J forums see http://www.jsoftware.com/forums.htm
>
----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm

Reply via email to