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 <tikk...@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 <
programm...@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

----------------------------------------------------------------------
For information about J forums see http://www.jsoftware.com/forums.htm

Reply via email to