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

gradient gives incorrect results on Julia 1.10 unless --check-bounds=yes #2192

Open
danielwe opened this issue Dec 9, 2024 · 2 comments
Open

Comments

@danielwe
Copy link
Contributor

danielwe commented Dec 9, 2024

Couldn't figure out why Pkg.test() works but include("test/runtests.jl") errors on the "Type unsstable return" testset. Turns out it's because Pkg.test() sets --check-bounds=yes, with which Enzyme.gradient sometimes gives completely different results than without, on Julia 1.10. No bounds error is emitted in either case.

On Julia 1.11 the value is not dependent on --check-bounds, but is different from both values obtained with Julia 1.10.

MWE (adapted from testset):

@show VERSION
@show Base.JLOptions().check_bounds

using Enzyme
using Random

const SEED = 42
const N_SAMPLES = 500
const N_COMPONENTS = 4

const rnd = Random.MersenneTwister(SEED)
const data = randn(rnd, N_SAMPLES)
const params0 = [rand(rnd, N_COMPONENTS); randn(rnd, N_COMPONENTS); 2rand(rnd, N_COMPONENTS)]

normal_pdf(x, mean, var) = exp(-(x - mean)^2 / (2var)) / sqrt(2π * var)

function mixture_loglikelihood1(params::AbstractVector{<:Real}, data::AbstractVector{<:Real})::Real
    K = length(params) ÷ 3
    weights, means, stds = @views params[1:K], params[K+1:2K], params[2K+1:end]
    mat = normal_pdf.(data, means', stds' .^2) # (N, K)
    sum(mat .* weights', dims=2) .|> log |> sum
end

const objective1 = params -> mixture_loglikelihood1(params, data)

grad = Enzyme.gradient(Reverse, objective1, params0)[1]
@show grad

Output with and without --check-bounds:

$ julia +1.10 --project=@enzyme --startup-file=no issue.jl
VERSION = v"1.10.7"
(Base.JLOptions()).check_bounds = 0
grad = [1361.9761422222543, 489.890298329022, 355.94097363693476, 292.0612340227955, -37.52901402937319, 69.1341952696743, -2.15332266254723, 37.98749089573396, -138.76969120534892, -30.81957613844974, -58.49460248865094, 12.87712891527131]
$ julia +1.10 --project=@enzyme --startup-file=no --check-bounds=yes issue.jl
VERSION = v"1.10.7"
(Base.JLOptions()).check_bounds = 1
grad = [289.7308495620467, 199.27559524985728, 236.6894577756876, 292.0612340227955, -9.429799389881452, 26.722295646439047, -1.9180355546752244, 37.98749089573396, -24.095620148778277, -13.935687326484112, -38.00044665702692, 12.87712891527131]

Zygote confirms that the result with --check-bounds=yes is the correct one. This is also what the testset assumes.

@danielwe
Copy link
Contributor Author

danielwe commented Dec 9, 2024

Turns out the changes from 1.10 to 1.11 are due to changes in the Random module, resulting in different data despite identical seed. With identical data, 1.11 gives the correct result with or without --check-bounds.

In other words, only 1.10 with --check-bounds=no has a problem.

@danielwe danielwe changed the title gradient gives different results depending on --check-bounds and Julia version gradient gives different results depending on --check-bounds with Julia 1.10 Dec 9, 2024
@danielwe
Copy link
Contributor Author

danielwe commented Dec 9, 2024

...and the changes in Random were of course dealt with in #2059, so if I had based my MWE on a more up-to-date version runtests.jl I'd have avoided that red herring.

The issue with 1.10 and --check-bounds=no remains.

@danielwe danielwe changed the title gradient gives different results depending on --check-bounds with Julia 1.10 gradient gives incorrect results on Julia 1.10 unless --check-bounds=yes Dec 10, 2024
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

No branches or pull requests

1 participant