Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCD (Greatest Common Divisior) #515

Closed
aman-godara opened this issue Sep 6, 2021 · 10 comments · Fixed by #539
Closed

GCD (Greatest Common Divisior) #515

aman-godara opened this issue Sep 6, 2021 · 10 comments · Fixed by #539
Assignees
Labels
easy Difficulty level is easy and good for starting into this project good first issue Good for newcomers idea Proposition of an idea and opening an issue to discuss it topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ...

Comments

@aman-godara
Copy link
Member

As suggested in #365, providing gcd function out of the box will be a good addition.

Description

Have a look at these comments to know more:
#365 (comment)
j3-fortran/fortran_proposals#221 (comment)

Prior Art

Many languages provide gcd out of the box.

@aman-godara aman-godara added the idea Proposition of an idea and opening an issue to discuss it label Sep 6, 2021
@aman-godara

This comment has been minimized.

@awvwgk awvwgk added easy Difficulty level is easy and good for starting into this project good first issue Good for newcomers labels Sep 6, 2021
@awvwgk awvwgk added the topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ... label Sep 18, 2021
@Carltoffel
Copy link
Member

Do you think it would be a good idea to split gcd into one gcd which swaps the arguments if necessary and another gcd which requires the first argument to be the greater one? I wonder if this could be used in some situations to make the routine a little bit faster.
On the one hand it will just be a tiny performance improvement (if any), on the other hand this would be very easy to implement (gcd with swap calls gcd without swap).

@arjenmarkus
Copy link
Member

arjenmarkus commented Sep 22, 2021 via email

@Sideboard
Copy link
Contributor

I would implement this to get started with stdlib.

@Sideboard
Copy link
Contributor

CC from PR:

Is there interest in a version of gcd that works with a rank-1 array for more than two arguments?

@ivan-pi
Copy link
Member

ivan-pi commented Sep 27, 2021

I've used gcd only once before in a derived-type mimicking rational numbers so I can't really estimate wider interest. In that case I would have been happy with the scalar version.

One thing I would point out is the similarity with some of the intrinsic functions, i.e. min(a, b[, c, [d...]]) which accepts two or more scalar arguments, and minval which seeks the minimum value (along a dimension) in an array. Maybe gcdval would be more fitting for the rank-1 array case? (To generalize to all possible dimensions and ranks takes a bit more effort with the preprocessor.)

Currently, the standard does not allow writing variadic functions like min but you can mimic it with optional arguments. I see Python 3.9 added support for an arbitrary number of arguments in math.gcd(*integers).

@aslozada
Copy link
Member

Maybe gcdval would be more fitting for the rank-1 array case?

Two cases with arrays:
R https://rdrr.io/cran/DescTools/man/GCD.html
GCD(..., na.rm=FALSE)

Wolfram https://reference.wolfram.com/language/ref/GCD.html
GCD[n1,n2,n3,...]

@ivan-pi
Copy link
Member

ivan-pi commented Sep 27, 2021

If we only plan to support two scalars or a vector I'm fine with putting these under the same interface like this:

interface gcd
  elemental function gcd_scalar(a,b) result(res)
    integer, intent(in) :: a, b
    integer :: res
  end function
  function gcd_vector(v) result(res)
    integer, intent(in) :: v(:)
    integer :: res
  end function
end interface

But as a language built on the foundation of arrays, I think a transformational function like shown below, fits well with existing Fortran intrinsics:

function gcdval(array,dim,mask) result(res)
  integer, intent(in) :: array(:,:)
  integer, intent(in) :: dim
  logical, intent(in) :: mask(:,:)
  integer :: res(:)
end function

Maybe @FortranFan is willing to share his opinion on what is the value/need for the val postfix and flexible dimension selection/masking? If gcd where to be proposed to J3 (and they did not reject it under the premise that users can write it themselves) what kind of expectations would there be from an array version?

@milancurcic
Copy link
Member

I've never used gcd. The closest to it I did was getting all common denominators and that was in Python. So I can only suggest things here based on what seems would be a nice API, and not any experience.

I don't know whether a gcd with an arbitrary number of inputs is more useful than the gcd with just two. But it would be more general, so even if such need is small, a gcd_vector would be more useful than gcd_scalar. Then the gcd_scalar becomes just a convenience API if somebody doesn't want to wrap a and b into an array, i.e. gcd([a, b]). So, I like the generic gcd that wraps gcd_scalar and gcd_vector.

gcdval also seems nice (consistent API with some intrinsics), assuming that it's useful. Let's discuss that one in a separate issue and PR as it's a different function really.

@ivan-pi
Copy link
Member

ivan-pi commented Oct 12, 2021

There were two posts about the GCD algorithm in Hacker News recently:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy Difficulty level is easy and good for starting into this project good first issue Good for newcomers idea Proposition of an idea and opening an issue to discuss it topic: mathematics linear algebra, sparse matrices, special functions, FFT, random numbers, statistics, ...
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants