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

FEAT: transparent round/to x 0 #4885

Merged
merged 3 commits into from
May 13, 2021
Merged

FEAT: transparent round/to x 0 #4885

merged 3 commits into from
May 13, 2021

Conversation

hiiamboris
Copy link
Collaborator

Following the discussion ☝️ April 22, 2021 9:27 PM and TG chat.

Summary

Rationale: to not create special cases where they aren't required

Example:

>> round/to pi 1
== 3
>> round/to pi 1e-10
== 3.1415926536
>> round/to pi 1e-100
== 3.141592653589793
>> round/to pi 1e-500
*** Math Error: math or number overflow
>> round/to pi 0
*** Math Error: math or number overflow

As scale approaches zero we should expect the result to come closer and closer to the original value.
With the lim (round/to value scale) = value when scale -> 0.
The zero can be reached in practice as mentioned in the above discussion, so it makes sense to handle it automatically, leaving programmer with less headaches.

This PR implements x = round/to x 0 invariant, even for special floats:

>> round/to pi 0
== 3.141592653589793
>> round/to now/time 0
== 21:10:46
>> round/to 1.#inf 0
== 1.#INF
>> round/to -1.#inf 0
== -1.#INF
>> round/to 1.#nan 0
== 1.#NaN

@hiiamboris hiiamboris changed the title FEAT: transparent round/to x 0 #4884 FEAT: transparent round/to x 0 Apr 24, 2021
@dockimbel dockimbel requested review from greggirwin and qtxie May 4, 2021 11:25
@dockimbel
Copy link
Member

This PR is changing the meaning of the /to refinement for round. The current meaning is:

REFINEMENTS:
     /to          => Return the nearest multiple of the scale parameter.
        scale        [number! money! time!] "Must be a non-zero value."

This PR should at least update the doc string accordingly.

@hiiamboris
Copy link
Collaborator Author

hiiamboris commented May 4, 2021

Thanks! Fixed!

n [number! money! time! pair!]
/to "Return the nearest multiple of the scale parameter"
scale [number! money! time!] "If zero, returns N unchanged"

Copy link
Contributor

@greggirwin greggirwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All OK by me.

@dockimbel
Copy link
Member

@qtxie Are you OK with the changes made by this PR?

@qtxie
Copy link
Contributor

qtxie commented May 10, 2021

@dockimbel Yes. I'm OK.

@dockimbel dockimbel merged commit 5d93987 into red:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants