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

Parse fractions properly and implement GMP #150

Open
dkahle opened this issue May 24, 2017 · 14 comments
Open

Parse fractions properly and implement GMP #150

dkahle opened this issue May 24, 2017 · 14 comments
Assignees
Labels

Comments

@dkahle
Copy link
Collaborator

dkahle commented May 24, 2017

> m2("1/3")
[1] "1/3"
> m2_parse(m2("1/3"))
[1] 1

Shouldn't be too hard, just check next token for / when you think you're looking at an int.

@dkahle dkahle added the bug label May 24, 2017
@coneill-math
Copy link
Owner

Running 1/3 in R yields a numeric. Should we do the same here? Technically this is a rational, but R does not have native support for those...

@coneill-math coneill-math self-assigned this May 24, 2017
@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

R's gmp package connects to gmp and would allow us to better address the problem; but this is actually part of a bigger issue. I kind of hate to add a dependency, but maybe we should require gmp and then parse things accordingly. Thoughts?

@coneill-math
Copy link
Owner

How would gmp help with this? It gives us more precision (helps with overflow, etc) but doesn't help with the fact that rationals dont exist in R.

Could we implement a basic "rational" data type, essentially a vector with 2 integers (numerator and denominator) with overloaded +, *, etc?

@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

gmp provides one with the as.bigq() function. For example:

> as.bigq(1L, 2L)
Big Rational ('bigq') :
[1] 1/2
> as.bigq(3L, 6L)
Big Rational ('bigq') :
[1] 1/2

(Arithmetic operators are also defined for the class.)

@coneill-math
Copy link
Owner

Ah, I see. This definitely looks like the way to go. If you add the dependency, I will fix the parsing.

Do we want to use this to store larger precision integers/floats as well? Is there a disadvantage to doing so?

@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

Sure, I'll do that shortly. gmp seems portable; I just installed it no problem on my Windows box. So... seems like a reliable resource. It's been around for a long time, and obviously the underling gmp library is widely used.

@coneill-math
Copy link
Owner

Agreed. What I mean is, do we want to use gmp for ALL of our integers and floating points, since Macaulay2 doesn't discriminate based on required precision?

@sommars
Copy link
Collaborator

sommars commented May 24, 2017

From a mathematical standpoint, that seems nice. Would that be natural to a statistician though?

@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

If possible, I think it'd be nice to have a logical global option set that governs this, like what you get with get_m2_port() or get_m2_path(). So, something like using_gmp():

> getOption("m2r")
$m2_path
[1] "/Applications/Macaulay2-1.9.2/bin"

> m2r:::set_m2r_option(gmp = TRUE) # we would make this into something like m2_use_gmp()
> getOption("m2r")
$m2_path
[1] "/Applications/Macaulay2-1.9.2/bin"

$gmp
[1] TRUE
> using_gmp <- function() getOption("m2r")$gmp
> using_gmp()
[1] TRUE

How hard do you think that'd be to implement? We may have to distinguish between Z's and Q's?

@coneill-math
Copy link
Owner

I agree this would be a nice option.

One weird thing: even if they disable gmp, they would still need to have it installed. Also, how would we handle fractions if they disable it?

@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

You mean if they set it to FALSE? I think they'd need to still have it installed, yes, but I'm not aware of any impediments to having it installed. (E.g. platform issues.)

If it's disabled, it makes most sense to me to just evaluate the fractions R style. So, something like:

string <- "1/3"
out <- as.numeric(str_split(string, "/")[[1]])
out[1] / out[2]
# [1] 0.3333333

@dkahle
Copy link
Collaborator Author

dkahle commented May 24, 2017

Also, I am inclined to think that gmp should be turned off by default. Since gmp numbers print differently than ordinary numbers, I don't want to users to be scared away by the printing. I also don't know how nicely gmp objects play with the rest of the R ecosystem. Thoughts?

To let users know it's available, we can randomly add a package startup message to the effect of m2r tip: Type use_gmp() to allow for multiple precision arithmetic.

dkahle added a commit to dkahle/m2r that referenced this issue May 29, 2017
dkahle added a commit that referenced this issue May 29, 2017
@coneill-math coneill-math changed the title Parse fractions properly Parse fractions properly and implement GMP Jun 14, 2017
@dkahle dkahle closed this as completed in 63e6075 Jun 15, 2017
@dkahle
Copy link
Collaborator Author

dkahle commented Jun 15, 2017

library(m2r)
# Loading required package: mpoly
# Loading required package: stringr
# please cite mpoly if you use it; see citation("mpoly")
# M2 found in /Applications/Macaulay2-1.9.2/bin
mat <- matrix(1:6, 3)
m2_matrix(mat)
# Starting M2... done.
#      [,1] [,2]
# [1,]    1    4
# [2,]    2    5
# [3,]    3    6
# M2 Matrix over ZZ[]
m2_toggle_gmp()
# m2r is now using gmp.
# m2_matrix(mat)
#      [,1] [,2]
# [1,]    1    4
# [2,]    2    5
# [3,]    3    6
# M2 Matrix over ZZ[]
m2_matrix(mat) %>% str
#  'm2_matrix' int [1:3, 1:2] 1 2 3 4 5 6
#  - attr(*, "m2_name")= chr "m2rintmatrix00000003"
#  - attr(*, "m2_meta")=List of 1
#   ..$ ring:Classes 'm2_polynomialring', 'm2'  atomic [1:1] NA
#   .. .. ..- attr(*, "m2_name")= chr "ZZ"
#   .. .. ..- attr(*, "m2_meta")=List of 3
#   .. .. .. ..$ vars    : NULL
#   .. .. .. ..$ coefring: chr "ZZ"
#   .. .. .. ..$ order   : chr "grevlex"

@dkahle dkahle reopened this Jun 15, 2017
@coneill-math
Copy link
Owner

Perhaps we should open a new issue for this? Something along the lines of "Improve GMP support"?

@coneill-math coneill-math assigned dkahle and unassigned coneill-math Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants