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

Simplify epred #323

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Simplify epred #323

merged 2 commits into from
Sep 12, 2024

Conversation

seabbs
Copy link
Contributor

@seabbs seabbs commented Sep 12, 2024

This PR is unlinked to an issue. In reviewing the current functionality needed for a family I slightly simplified the current code.

Unrelated to this PR there are lots of ways we can streamline adding families and sharing support across models and across families (for models) it looks like which is great.

@seabbs seabbs requested a review from athowes September 12, 2024 13:53
@seabbs
Copy link
Contributor Author

seabbs commented Sep 12, 2024

Hmm interesting. The with approch is directly based on the lognormal code from brms so curious why this fails tests (but not otherwise?)

@athowes
Copy link
Collaborator

athowes commented Sep 12, 2024

Hmm interesting. The with approch is directly based on the lognormal code from brms so curious why this fails tests (but not otherwise?)

Yeah not sure, I guess just look at prep$dpar and see if it looks as expected. Maybe it doesn't for some reason

@athowes
Copy link
Collaborator

athowes commented Sep 12, 2024

I'll go in and look at this.

@athowes
Copy link
Collaborator

athowes commented Sep 12, 2024

> str(prep$dpars)
List of 2
 $ mu   :List of 10
  ..$ family:List of 16
  .. ..$ family           : chr "custom"
  .. ..$ link             : chr "identity"
  .. ..$ name             : chr "latent_lognormal"
  .. ..$ dpars            : chr [1:2] "mu" "sigma"
  .. ..$ lb               :List of 2
  .. .. ..$ mu   : chr NA
  .. .. ..$ sigma: chr "0"
  .. ..$ ub               :List of 2
  .. .. ..$ mu   : chr NA
  .. .. ..$ sigma: chr NA
  .. ..$ type             : chr "real"
  .. ..$ vars             : chr [1:3] "pwindow" "swindow" "vreal1"
  .. ..$ loop             : logi FALSE
  .. ..$ specials         : chr(0) 
  .. ..$ log_lik          : NULL
  .. ..$ posterior_predict: NULL
  .. ..$ posterior_epred  : NULL
  .. ..$ env              :<environment: 0x12f030b38> 
  .. ..$ normalized       : chr ""
  .. ..$ link_sigma       : chr "log"
  .. ..- attr(*, "class")= chr [1:3] "customfamily" "brmsfamily" "family"
  ..$ ndraws: int 4000
  ..$ nobs  : int 500
  ..$ fe    :List of 2
  .. ..$ X: num [1:500, 1] 1 1 1 1 1 1 1 1 1 1 ...
  .. .. ..- attr(*, "dimnames")=List of 2
  .. .. .. ..$ : chr [1:500] "1" "2" "3" "4" ...
  .. .. .. ..$ : chr "Intercept"
  .. .. ..- attr(*, "assign")= int 0
  .. ..$ b: num [1:4000, 1] 1.75 1.77 1.73 1.76 1.76 ...
  .. .. ..- attr(*, "dimnames")=List of 2
  .. .. .. ..$ draw    : chr [1:4000] "1" "2" "3" "4" ...
  .. .. .. ..$ variable: chr "b_Intercept"
  .. .. ..- attr(*, "nchains")= int 4
  ..$ sp    : list()
  ..$ cs    : list()
  ..$ sm    : list()
  ..$ gp    : list()
  ..$ re    : list()
  ..$ ac    : list()
  ..- attr(*, "class")= chr "bprepl"
 $ sigma: num [1:4000] 0.498 0.502 0.486 0.469 0.488 ...

@athowes
Copy link
Collaborator

athowes commented Sep 12, 2024

I think because it's a custom family it's more complicated, so reverting back to the get_dpars version (and keeping the line saving in the gamma case).

@seabbs
Copy link
Contributor Author

seabbs commented Sep 12, 2024

Oh thats very interesting. Good detective work

@seabbs seabbs enabled auto-merge (squash) September 12, 2024 17:00
@athowes athowes self-requested a review September 12, 2024 18:53
@seabbs seabbs disabled auto-merge September 12, 2024 19:27
@seabbs seabbs merged commit 8f441d3 into main Sep 12, 2024
7 checks passed
@seabbs seabbs deleted the use-with branch September 12, 2024 19:27
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.

2 participants