[Jprogramming] Error in comment in addons/math/misc/pollard.ijs

[Henry Rich]

Like Ric said, the pull request makes things easier for the maintainers 
and makes your request less likely to be forgotten.  You could feel free 
to make the comments in the Forum as well.

Hwenry Rich

On 2/13/2021 6:49 PM, 'Mike Day' via Programming wrote:
> Thanks, Ric.
>
> I think I’d prefer to continue offering public comments open to criticism and correction in this and perhaps other forums/fora.
>
>   Cheers,
>
> Mike
>
> Sent from my iPad
>
>> On 13 Feb 2021, at 23:20, Ric Sherlock <tikkanz at gmail.com> wrote:
>>
>> 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 <
>> programming at jsoftware.com> 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
> ----------------------------------------------------------------------
> For information about J forums see http://www.jsoftware.com/forums.htm


-- 
This email has been checked for viruses by AVG.
https://www.avg.com