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

Obscure regression in 32-bit Julia 0.6.3 #27345

Closed
nalimilan opened this issue May 31, 2018 · 8 comments
Closed

Obscure regression in 32-bit Julia 0.6.3 #27345

nalimilan opened this issue May 31, 2018 · 8 comments
Labels
regression Regression in behavior compared to a previous version system:32-bit Affects only 32-bit systems
Milestone

Comments

@nalimilan
Copy link
Member

In Julia 0.6.3, StatsBase.wmedian gives incorrect results on 32-bit. In the simplified function below, the result is 8.5 on 0.6.2 and 0.7.0-alpha, but 10.0 on 0.6.3. Everything is OK on 64-bit.

FWIW, the bug goes away just by adding @show statements.

function wmedian(v::AbstractVector{<:Real}, wt::AbstractVector{<:Real})
    midpoint = sum(wt)/2
    maxval, maxind = findmax(wt)
    if maxval > midpoint
        v[maxind]
    else
        permute = sortperm(v)
        cumulative_weight = zero(eltype(wt))
        i = 0
        for (_i, p) in enumerate(permute)
            i = _i
            if cumulative_weight == midpoint
                i += 1
                break
            elseif cumulative_weight > midpoint
                cumulative_weight -= wt[p]
                break
            end
            cumulative_weight += wt[p]
        end
        if cumulative_weight == midpoint
            middle(v[permute[i-2]], v[permute[i-1]])
        else
            middle(v[permute[i-1]])
        end
    end
end

julia> wmedian([1, 2, 4, 7, 10, 15], [1/3, 1/3, 1/3, 1, 1, 1])
10.0
@nalimilan nalimilan added the regression Regression in behavior compared to a previous version label May 31, 2018
@ViralBShah ViralBShah added this to the 0.6.x milestone May 31, 2018
@pkofod
Copy link
Contributor

pkofod commented May 31, 2018

Our 32bit tests also fail on some weird last digit difference in LineSearches.jl. I can report details later (on mobile) but if I hardcode copies of the variables involved the results are correct.

edit:
I have not looked at making this self-contained, but to show the problem I had the following in a function

    a = ((phitest-phi_0)/alphatest - dphi_0)/alphatest  # quadratic fit
   @show a
   _a = 0.72
   _b = 2.0
   _c = 0.2
   _d = -8.0
   @show _a === phitest
   @show _b === phi_0
   @show _c === alphatest
   @show _d === dphi_0
   @show ((_a - _b)/_c - _d)/_c
   @show ((phitest-phi_0)/alphatest - dphi_0)/alphatest

with the output

a = 8.0
_a === phitest = true
_b === phi_0 = true
_c === alphatest = true
_d === dphi_0 = true
((_a - _b) / _c - _d) / _c = 8.000000000000002
((phitest - phi_0) / alphatest - dphi_0) / alphatest = 8.0

! Pretty weird that they can be identical ===, but the two calculations return different values!

@ararslan ararslan added the system:32-bit Affects only 32-bit systems label May 31, 2018
@ChrisRackauckas
Copy link
Member

Our 32-bit tests are failing because 0.5:0.1:0.7 is becoming 0.5:0.1:0.6. Maybe that's a small case to debug.

https://ci.appveyor.com/project/YingboMa/ordinarydiffeq-jl/build/1.0.62/job/o113qdrx1owjjfy2#L458

@ararslan
Copy link
Member

ararslan commented Jun 1, 2018

Would someone with access to a 32-bit system be willing to run git bisect between v0.6.2 and v0.6.3? git bisect run with a minimal reproducing test case would make it easier. I'd do it myself but all of the machines I have access to are 64-bit.

Sorry this didn't get caught. PackageEvaluator runs on 64-bit Ubuntu so it doesn't catch OS- or architecture-specific issues in packages.

@nalimilan
Copy link
Member Author

I have tried replicating the problem on a Fedora 32-bit VM, and while I see it with the official binaries, I don't when building from source. I'm using MARCH=pentium4, which AFAICT is what the official binaries use too. I'm not sure where the difference can come from.

For post-1.0 minor releases, we really need 32-bit PackageEvaluator runs (and ideally on several platforms, including Windows and OS X).

@KristofferC
Copy link
Member

Any difference in the generated code? Julia lowering, llvm code, generated code?

@nalimilan
Copy link
Member Author

Good question. There are lots of small differences in the native code, but none in the LLVM IR. So it seems it's a bug in LLVM? See this gist, where the first "commit" contains results with the official binaries, and the second one results with my custom build.

@ararslan
Copy link
Member

ararslan commented Jun 4, 2018

See #27402. New 32-bit binaries for Windows and Linux have been uploaded. Let me know if this fixes things for you.

@nalimilan
Copy link
Member Author

Thanks, that did the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Regression in behavior compared to a previous version system:32-bit Affects only 32-bit systems
Projects
None yet
Development

No branches or pull requests

6 participants