add Grouping contrasts #339

kleinschmidt · 2020-07-13T23:15:37Z

This is in lieu of having these defined in StatsModels.jl; as we've discussed
they may be a bit to specialized for the specific demands of grouping variables
in MixedModels without defining enough functionality to be of use in other
settings, so it makes sense to have them here until some kind of sensible
handling of grouping is incorporated into StatsModels.

The Grouping contrasts are a "pseudocontrast" that never defines a contrasts
matrix, just keeps track of the unique values and an "inverse index" which maps
levels to a numeric index. This allows you to get around the OutOfMemoryError
that's triggered when you have a grouping variable with a very large number of
levels. The catch is that, at the moment, you have to manually specify the
grouping contrasts when constructing the model. I've added a guard to catch OOM
errors during schema construction which prints a warning, but we could also
throw a different error there instead to be clearer. The downside with that
would be that if it's NOT a problem with the ranef grouping term, then they'd
just get useless advice and no useful stacktrace, so that's why I went with
printing a warning.

kleinschmidt · 2020-07-13T23:23:14Z

based on JuliaStats/StatsModels.jl#181

codecov · 2020-07-13T23:26:25Z

Codecov Report

Merging #339 into master will decrease coverage by 0.12%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
- Coverage   95.75%   95.63%   -0.13%     
==========================================
  Files          22       23       +1     
  Lines        1485     1512      +27     
==========================================
+ Hits         1422     1446      +24     
- Misses         63       66       +3

Impacted Files	Coverage Δ
src/MixedModels.jl	`100.00% <ø> (ø)`
src/linearmixedmodel.jl	`99.15% <50.00%> (-0.85%)`	⬇️
src/grouping.jl	`60.00% <60.00%> (ø)`
src/remat.jl	`94.94% <0.00%> (-0.06%)`	⬇️
src/linalg/rankUpdate.jl	`98.43% <0.00%> (+0.76%)`	⬆️
src/arraytypes.jl	`100.00% <0.00%> (+4.87%)`	⬆️

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 6fabb48...4356ad1. Read the comment docs.

palday

The catch is that, at the moment, you have to manually specify the
grouping contrasts when constructing the model. I've added a guard to catch OOM
errors during schema construction which prints a warning, but we could also
throw a different error there instead to be clearer.

I think this is the way to go for now, but we should consider automatically setting the contrasts on grouping variables and doing this separately from the fixed-effects contrasts. This would allow us a bit of magic that could be good or bad, namely having a discrete, yet numeric fixed effect that also appears as a grouping variable. You can think of this as an approximation for a GAM (it's not quite the same as a GAM, but GAMs can indeed be rewritten as mixed models). Right now, you can of course still do this by having two copies of the same variable, one categorical and one numeric. For our planned neuro time-series stuff, this is actually surprisingly relevant because we time as a discrete numeric variable and expect non linear temporal trends.

src/grouping.jl

palday · 2020-07-14T10:05:01Z

src/linearmixedmodel.jl

+        if isa(e, OutOfMemoryError)
+            @warn "Random effects grouping variables with many levels can cause out-of-memory errors.  Try manually specifying `Grouping()` contrasts for those variables."
+        end
+        rethrow(e)


I think this is the way to go right now, but I'm thinking about whether we should automatically set Grouping contrasts for grouping variables.

Co-authored-by: Phillip Alday <palday@users.noreply.github.com>

kleinschmidt · 2020-07-14T14:35:14Z

Yeah I agree @palday, it would be nice to be able to do this automatically. It actually may be possible now but would take some annoying fiddling; you'd have to unpack the FunctionTerm{typeof(|)}, or conver them to ReTerms in order to figure out what the grouping variables are before constructing the schema, because that's when the contrasts are needed to avoid the OOM. This would basically amount to circumventing the normal StatsModels pipeline which is why I'd proposed somewhere (JuliaStats/StatsModels.jl#154) to add an apply_context stage where you don't have the schema yet but you do have the context in which the formula will be used, so you could do things like create all the ReTerms you need before creating the schema.

kleinschmidt · 2020-07-14T15:04:01Z

I think at least some of the CI failures are not unrelated to this PR but have to do with how the OS handles OOM. The MacOS one failed before too, apparently during the tests for the added functionality where I intentionally trigger an OOM error. And the cancelled jobs might be related to this as well, but I'm not sure, although the passing tests on at least one windows build tells me that OOM doesn't always result in a canceled job. I guess the thing to do is to remove the tests which intentionally cause an OOM because otherwise we'll always have failing CI which...defeats the purpose.

palday · 2020-07-14T15:43:15Z

@kleinschmidt The failures on Future are a known issue. For Windows .... I'll have to look at the details of the GitHub Action / its docker image to figure out why it just cancels. For OS X: do you have a local Mac?

kleinschmidt · 2020-07-14T17:59:40Z

I'm afraid not.

kliegl · 2020-07-14T19:03:18Z

I'm afraid not. @palday @kleinschmidt

I have many local macs

kleinschmidt · 2020-07-14T20:00:58Z

@kliegl can you check out this PR branch and run the tests? The procedure is to

] dev MixedModels
] activate ~/.julia/dev/MixedModels
; git checkout dfk/grouping
] test

kliegl · 2020-07-14T20:20:08Z

@kliegl can you check out this PR branch and run the tests? The procedure is to
] dev MixedModels
] activate ~/.julia/dev/MixedModels
; git checkout dfk/grouping
] test

The ;git checkout dfk/grouping fails. Do I first have to clone this?

kleinschmidt · 2020-07-14T20:26:57Z

ah yeah you'll need to first go to that directory, with

; cd .julia/dev/MixedModels
; git fetch origin && git checkout dfk/grouping

kliegl · 2020-07-14T20:30:40Z

It is running now. I just had to break it into two commands.

kliegl · 2020-07-14T20:45:06Z

ERROR: Package MixedModels errored during testing (received signal: KILL)

Test_2020-07-14.txt

kleinschmidt · 2020-07-14T21:04:49Z

Yeah looks from that like the process is just killed when it hits the OOM tests. I'll push a change that only tests the good path (I'm sufficiently convinced that trying to construct a 1,000,000x1,000,000 dense matrix will OOM...)

kliegl · 2020-07-14T22:05:17Z

Just ran it again:
Testing MixedModels tests passed

Do you need the report?

kleinschmidt · 2020-07-14T23:37:15Z

Looks like the CI now is failing because of a problem with the artifacts...

palday · 2020-07-15T10:46:32Z

@kleinschmidt Yeah, I'm still sorting out the artifact link stuff. Right now, the default link always points to the newest version, but that of course doesn't match the SHA for older versions. I'll see if I can't fix that as part of #341

…k/grouping

palday · 2020-07-15T18:13:55Z

@kleinschmidt assuming that all the CIs (potentially except Future) pass, I think this is good to go.

* add Grouping contrasts * export and add a getproperty patch to work around statsmodels * warn users to use Grouping when OOM during schema call * Update src/grouping.jl Co-authored-by: Phillip Alday <phillip.alday@mpi.nl> (cherry picked from commit 2f9d423)

* add Grouping contrasts (#339) Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

kleinschmidt added 3 commits July 13, 2020 18:52

add Grouping contrasts

2dd66be

export and add a getproperty patch to work around statsmodels

63feeda

warn users to use Grouping when OOM during schema call

600ebc7

kleinschmidt requested review from dmbates and palday July 13, 2020 23:15

palday approved these changes Jul 14, 2020

View reviewed changes

Update src/grouping.jl

7744d5c

Co-authored-by: Phillip Alday <palday@users.noreply.github.com>

remove tests that actually throw OOM

e46d273

kleinschmidt mentioned this pull request Jul 14, 2020

use DataAPI.levels instead of levels field on contrasts, terms as hints JuliaStats/StatsModels.jl#181

Merged

palday added 2 commits July 15, 2020 16:47

use older artifact URL

67322d9

Merge branch 'master' of github.com:JuliaStats/MixedModels.jl into df…

4356ad1

…k/grouping

palday approved these changes Jul 15, 2020

View reviewed changes

dmbates merged commit 2f9d423 into master Jul 15, 2020

kleinschmidt mentioned this pull request Jul 30, 2020

Strange OutOfMemoryError() #348

Closed

palday added a commit that referenced this pull request Aug 12, 2020

Grouping for 2.x (#353)

dcd9242

* add Grouping contrasts (#339) Co-authored-by: Dave Kleinschmidt <dave.f.kleinschmidt@gmail.com>

palday deleted the dfk/grouping branch November 19, 2020 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Grouping contrasts #339

add Grouping contrasts #339

kleinschmidt commented Jul 13, 2020

kleinschmidt commented Jul 13, 2020 •

edited

codecov bot commented Jul 13, 2020 •

edited

palday left a comment

palday Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

palday commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020 •

edited

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

palday commented Jul 15, 2020

palday commented Jul 15, 2020

add Grouping contrasts #339

add Grouping contrasts #339

Conversation

kleinschmidt commented Jul 13, 2020

kleinschmidt commented Jul 13, 2020 • edited

codecov bot commented Jul 13, 2020 • edited

Codecov Report

palday left a comment

Choose a reason for hiding this comment

palday Jul 14, 2020

Choose a reason for hiding this comment

kleinschmidt commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

palday commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020 • edited

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

kliegl commented Jul 14, 2020

kleinschmidt commented Jul 14, 2020

palday commented Jul 15, 2020

palday commented Jul 15, 2020

kleinschmidt commented Jul 13, 2020 •

edited

codecov bot commented Jul 13, 2020 •

edited

kliegl commented Jul 14, 2020 •

edited