Skip to content

Conversation

@yakir12
Copy link
Contributor

@yakir12 yakir12 commented Dec 5, 2019

No description provided.

@yakir12
Copy link
Contributor Author

yakir12 commented Dec 5, 2019

While this looks good, I am getting these errors from trying this:

julia> fm = fit!(GeneralizedLinearMixedModel(@formula(homebound ~ 1 + Run + (1+Run|Id)), df, Normal(), SqrtLink()))
ERROR: ArgumentError: invalid NLopt arguments: zero step size
Stacktrace:
 [1] chk(::NLopt.Opt, ::NLopt.Result) at /home/yakir/.julia/packages/NLopt/eqN9a/src/NLopt.jl:237
 [2] initial_step! at /home/yakir/.julia/packages/NLopt/eqN9a/src/NLopt.jl:340 [inlined]
 [3] NLopt.Opt(::OptSummary{Float64}) at /home/yakir/.julia/packages/MixedModels/eXZQr/src/optsummary.jl:85
 [4] #fit!#59(::Bool, ::Bool, ::Int64, ::typeof(fit!), ::GeneralizedLinearMixedModel{Float64}) at /home/yakir/.julia/packages/MixedModels/eXZQr/src/generalizedlinearmixedmodel.jl:182
 [5] fit!(::GeneralizedLinearMixedModel{Float64}) at /home/yakir/.julia/packages/MixedModels/eXZQr/src/generalizedlinearmixedmodel.jl:160
 [6] top-level scope at REPL[9]:1

Maybe specifying a non identity link function is simply not cosher?

@palday
Copy link
Member

palday commented Dec 5, 2019

See #94, #95 .

@palday
Copy link
Member

palday commented Dec 5, 2019

The fix for the NLopt error (#201) hasn't landed yet because the test case we have for that depends on the gamma functionality. If we had a good test case that didn't depend on that, I would split the PR and go ahead and get the boundary fit issue in.

@codecov-io
Copy link

codecov-io commented Dec 5, 2019

Codecov Report

Merging #222 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #222   +/-   ##
=======================================
  Coverage   92.89%   92.89%           
=======================================
  Files          18       18           
  Lines        1295     1295           
=======================================
  Hits         1203     1203           
  Misses         92       92
Impacted Files Coverage Δ
src/generalizedlinearmixedmodel.jl 91.19% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 376bc71...38aa075. Read the comment docs.

@yakir12
Copy link
Contributor Author

yakir12 commented Dec 5, 2019

If we had a good test case that didn't depend on that, I would split the PR and go ahead and get the boundary fit issue in.

Can I help with that? I have data that isn't gamma related...

@palday
Copy link
Member

palday commented Dec 5, 2019

Yes, please. Esp if it's a small dataset (both for repository and speed-of-tests reasons).

@yakir12
Copy link
Contributor Author

yakir12 commented Dec 5, 2019

I am unsure exactly what this would entail, but here is the CSV for the tiny data-set:
https://s3.eu-central-1.amazonaws.com/vision-group-file-sharing/Data%20backup%20and%20storage/Yakir/coffee%20beetles/beetles.csv
Not sure what else you need. I can give a short explanation, or the code to test it, let me know...

@palday
Copy link
Member

palday commented Dec 5, 2019

The code to fit the model (so formula, family, link).

@yakir12
Copy link
Contributor Author

yakir12 commented Dec 5, 2019

A "simple" case:

using CSV, DataFrames, MixedModels
df = CSV.File("beetles.csv") |> DataFrame
df = df[!, Not(:outbound)]
fm = fit!(GeneralizedLinearMixedModel(@formula(homebound ~ 1 + Run + (1+Run|Id)), df, Normal(), LogLink()))

and more complicated:

df = CSV.File("beetles.csv") |> DataFrame
df = stack(df, variable_name = :Direction, value_name = :Speed)
fm = fit!(GeneralizedLinearMixedModel(@formula(Speed ~ 1 + Run * Direction + (1 + Run |Id)), df, Normal(), LogLink()))

@dmbates
Copy link
Collaborator

dmbates commented Dec 5, 2019

I think we should merge this change (it corrects an obvious error) and move the discussions to #94 and #95

@dmbates dmbates merged commit 9c7bb55 into JuliaStats:master Dec 5, 2019
@yakir12 yakir12 deleted the yakir branch January 7, 2020 09:18
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