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

math.round returns 1 for 0.49999999999999994 #14755

Closed
kromka-chleba opened this issue Jun 15, 2024 · 6 comments · Fixed by #14757
Closed

math.round returns 1 for 0.49999999999999994 #14755

kromka-chleba opened this issue Jun 15, 2024 · 6 comments · Fixed by #14757
Labels
Bug Issues that were confirmed to be a bug @ Script API

Comments

@kromka-chleba
Copy link
Contributor

kromka-chleba commented Jun 15, 2024

Problem

As pointed out below the most popular answer on stackoverflow, Minetest's implementation of math.round suffers from a bug: https://stackoverflow.com/questions/18313171/lua-rounding-numbers-and-then-truncate

> math.round(0.49999999999999994)
1

Solution

I rewrote math random this way:

function math.round(x)
    local int, frac = math.modf(x)
    if x >= 0 then
        if frac >= 0.5 then
            return int + 1
        else
            return int
        end
    else
        if frac <= -0.5 then
            return int - 1
        else
            return int
        end
    end
end

Old:

> math.round(0.49999999999999994)
1
> math.round(-0.49999999999999994)
-1
> math.round(0)
0
> math.round(10.3)
10
> math.round(10.5)
11
> math.round(-10.3)
-10
> math.round(-10.5)
-11

New:

> math.round(0.49999999999999994)
0
> math.round(-0.49999999999999994)
0
> math.round(0)
0
> math.round(10.3)
10
> math.round(10.5)
11
> math.round(-10.3)
-10
> math.round(-10.5)
-11

Steps to reproduce

Do this:

> math.round(0.49999999999999994)
1
@kromka-chleba kromka-chleba added the Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible label Jun 15, 2024
@kromka-chleba
Copy link
Contributor Author

I guess I can make the function shorter a little:

function math.round(x)
    local int, frac = math.modf(x)
    if x >= 0 and frac >= 0.5 then
        return int + 1
    elseif frac <= -0.5 then
        return int - 1
    end
    return int
end

@SmallJoker SmallJoker added the Upstream issue This bug is the fault of a library, the OS or an external service we use. label Jun 15, 2024
@kromka-chleba

This comment was marked as resolved.

@SmallJoker SmallJoker removed the Upstream issue This bug is the fault of a library, the OS or an external service we use. label Jun 15, 2024
@SmallJoker

This comment was marked as duplicate.

@appgurueu
Copy link
Contributor

I guess I can make the function shorter a little [...]

x >= 0 should be unnecessary, just frac >= 0.5 should suffice, no? I'd also probably use just early returns, no need for the elseif there.

This could even be written as a single line with and and or, but I don't think that is worthwhile.

It might be interesting to see how the performance compares (and whether there is anyone relying on rounding being particularly fast to begin with).

kromka-chleba added a commit to kromka-chleba/minetest that referenced this issue Jun 16, 2024
@kromka-chleba
Copy link
Contributor Author

Made a PR that fixes it.

@appgurueu appgurueu added Bug Issues that were confirmed to be a bug and removed Unconfirmed bug Bug report that has not been confirmed to exist/be reproducible labels Jun 16, 2024
kromka-chleba added a commit to kromka-chleba/minetest that referenced this issue Jun 16, 2024
kromka-chleba added a commit to kromka-chleba/minetest that referenced this issue Jun 19, 2024
@The4codeblocks
Copy link

this..... is painful
the number written in scientific notation for base 2 is practically 1.1111111111111111111111111111111111111111111111111111*10^-2
or in straight base 2
0.011111111111111111111111111111111111111111111111111111
IT'S PAINFUL
ROUND CAN BE IMPLEMENTED WITHIN PROCESSOR ARCHITECTURE (or its emulate) VERY EASILY, BUT THEY CHOSE NOT TO.
just a floor and a shift and an add.

E

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues that were confirmed to be a bug @ Script API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants