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