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

[Feature] Add report.compare.loo #419

Merged
merged 11 commits into from
Apr 1, 2024
Merged

[Feature] Add report.compare.loo #419

merged 11 commits into from
Apr 1, 2024

Conversation

DominiqueMakowski
Copy link
Member

> x <- brms::loo_compare(brms::add_criterion(m1, "loo"),
+                        brms::add_criterion(m2, "loo"),
+                        model_names=c("m1", "m2"))
> report(x)
The difference in predictive accuracy, as index by Expected Log Predictive Density (ELPD), suggests that 'm2' is the best model, followed by 'm1' (diff =
-9.70, Z-diff = -2.98)

@mattansb
Copy link
Member

Made some changes to report if LOO or WAIC was used and to allow for ELPD or IC to be reported, and reporting the effective number of parameters (p).

library(loo)
library(rstanarm)
library(report)

m1 <- stan_glm(mpg ~ 1, data = mtcars, refresh = 0)
m2 <- stan_glm(mpg ~ am, data = mtcars, refresh = 0)
m3 <- stan_glm(mpg ~ hp, data = mtcars, refresh = 0)

c.loo <- loo_compare(loo(m1), loo(m2), loo(m3))
c.waic <- loo_compare(waic(m1), waic(m2), waic(m3))

report(c.loo)
#> The difference in predictive accuracy, as index by Expected Log Predictive
#> Density (ELPD-LOO), suggests that 'm3' is the best model (effective number of
#> parameters (ENP) = 3.66), followed by 'm2' (diff = -7.05, ENP = 2.82, z-diff =
#> -2.39) and 'm1' (diff = -12.99, ENP = 1.80, z-diff = -3.77).
report(c.loo, index = "IC")
#> The difference in predictive accuracy, as index by Leave-One-Out CV Information
#> Criterion (LOOIC), suggests that 'm3' is the best model (effective number of
#> parameters (ENP) = 3.66), followed by 'm2' (diff = 14.10, ENP = 2.82, z-diff =
#> 2.39) and 'm1' (diff = 25.99, ENP = 1.80, z-diff = 3.77).

report(c.waic)
#> The difference in predictive accuracy, as index by Expected Log Predictive
#> Density (ELPD-WAIC), suggests that 'm3' is the best model (effective number of
#> parameters (ENP) = 3.52), followed by 'm2' (diff = -7.15, ENP = 2.78, z-diff =
#> -2.45) and 'm1' (diff = -13.12, ENP = 1.78, z-diff = -3.90).
report(c.waic, index = "IC")
#> The difference in predictive accuracy, as index by Widely Applicable
#> Information Criterion (WAIC), suggests that 'm3' is the best model (effective
#> number of parameters (ENP) = 3.52), followed by 'm2' (diff = 14.29, ENP = 2.78,
#> z-diff = 2.45) and 'm1' (diff = 26.24, ENP = 1.78, z-diff = 3.90).

Created on 2024-03-26 with reprex v2.1.0

@DominiqueMakowski
Copy link
Member Author

very cool! will be useful for teaching

@DominiqueMakowski
Copy link
Member Author

@rempsyc I think this is good to be merged

@rempsyc
Copy link
Member

rempsyc commented Mar 28, 2024

Thanks, but we see:

── R CMD check results ───────────────────────────────────── report 0.5.8.1 ────
Duration: 5m 4.3schecking Rd cross-references ... WARNING
  Missing link or links in documentation object 'report.compare.loo.Rd':
    ‘loo_compareSee section 'Cross-references' in the 'Writing R Extensions' manual.

❯ checking Rd \usage sections ... WARNING
  Undocumented arguments in documentation object 'report.compare.loo'
    ‘...’
  
  Functions with \usage entries need to have the appropriate \alias
  entries, and all their arguments documented.
  The \usage entries must correspond to syntactically valid R code.
  See chapterWriting R documentation filesin theWriting R
  Extensionsmanual.

  WORD        FOUND IN
elpd        report.compare.loo.Rd:22,23
ELPD        report.compare.loo.Rd:13
IC          report.compare.loo.Rd:13
Magnusson   report.compare.loo.Rd:23
Sivula      report.compare.loo.Rd:23
Vehtari     report.compare.loo.Rd:23
Error:
! Spelling errors found. Either correct them or update `inst/WORDLIST`
  using `spelling::update_wordlist()`.

── Building function reference ─────────────────────────────────────────────────
Error in `build_reference_index()`:
! All topics must be included in reference indexMissing topics: report.compare.loo
 Either add to _pkgdown.yml or use @keywords internal

@DominiqueMakowski
Copy link
Member Author

should be fixed

@rempsyc
Copy link
Member

rempsyc commented Mar 28, 2024

Thanks, but we still see:

  WORD        FOUND IN
elpd        report.compare.loo.Rd:22,23
ELPD        report.compare.loo.Rd:13
IC          report.compare.loo.Rd:13
Magnusson   report.compare.loo.Rd:23
Sivula      report.compare.loo.Rd:23
Vehtari     report.compare.loo.Rd:23
Error:
! Spelling errors found. Either correct them or update `inst/WORDLIST`
  using `spelling::update_wordlist()`.

── Building function reference ─────────────────────────────────────────────────
Error in `build_reference_index()`:
! All topics must be included in reference indexMissing topics: report.compare.loo
 Either add to _pkgdown.yml or use @keywords internal

Some files don't follow the style guide. Run styler::style_pkg() with latest {styler}.

Error: Process completed with exit code 1.

@DominiqueMakowski
Copy link
Member Author

image

@rempsyc
Copy link
Member

rempsyc commented Mar 30, 2024

Your wishes are my commands. Why haven't you started with this, Master Sidious? Let me get to work and fix all those remaining issues now

image

@rempsyc
Copy link
Member

rempsyc commented Mar 30, 2024

  • Error in build_reference_index()
  • Some files don't follow the style guide ✅
  • Lints ✅

Snapshots are still failing, but I'm afraid we'll have to fix them in a different PR :/

@rempsyc
Copy link
Member

rempsyc commented Mar 30, 2024

Can you update the readme NEWS.md with the addition of this new function? I don't know what it does 😬

@DominiqueMakowski
Copy link
Member Author

Can you update the readme

No need to put it in the README I'd say, it's quite a niche and mostly "convenience" feature (a tweet about it will suffice :)

@rempsyc
Copy link
Member

rempsyc commented Apr 1, 2024

Boy am I tired hehe, I meant the NEWS.md 😂 It is done now, I have updated it, sorry about that

@rempsyc rempsyc self-requested a review April 1, 2024 09:53
@rempsyc rempsyc merged commit 44c92ad into main Apr 1, 2024
20 of 27 checks passed
@rempsyc rempsyc deleted the report_loo branch April 1, 2024 10:33
@strengejacke
Copy link
Member

@rempsyc waiting for Dom to add tests and fix check issues...

image

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