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

Continuing development of ZIPLN models #118

Merged
merged 31 commits into from Mar 5, 2024
Merged

Continuing development of ZIPLN models #118

merged 31 commits into from Mar 5, 2024

Conversation

jchiquet
Copy link
Member

@jchiquet jchiquet commented Jan 24, 2024

  • Exact optimization with Lambert function for $R$ (parameter of the conditional distirbution of the ZI component)
  • Network-like methods for ZIPLNfit_sparse
  • ZIPLNnetwork, a
    • collection with PLNfamilly collecting a series of ZIPLNfit_sparse object with automatic grid
    • merge/share classes between PLNnetworkfamily and ZIPLNnetwork

Still missing

  • use correct weights in estimation of $\pi$ when using exact distributino of $W$ (with Lambert function)
  • complete test for ziplnnetwork

Note that I won't add postTreatment yet zo ZIPLN since I think we need to rethink a bit all the psotTreatment made on PLN-lie objects(in particular, all the variance estimators...)

Copy link
Collaborator

@mahendra-mariadassou mahendra-mariadassou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the 3 small comments:

  • inception_cov to constrain the shape of the covariance matrix in the inception model
  • the remark about lapply / future.lapply
  • number of parameters for ZIPLNnetwork (the extra p is indeed present in PLNnetwork)
    It's all good for me.

R/PLNnetwork.R Outdated
@@ -122,5 +122,6 @@ PLNnetwork_param <- function(
variance = TRUE ,
config_post = config_pst ,
config_optim = config_opt ,
### TODO CHECK: Why two inceptive model (cov and not ?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduced in ctrapnell's PR. Allows the user to:

  • specify a custom inception with inception
  • constrain the shape inception_cov of the covariance in the (yet-to-be fitted) inception model used to initialize members of the family among full (default, same as before), diagonal, spherical
    Useful for large dataset as the "full" model used for initialization (corresponding to $\lambda = 0$ can take a long time to fit.
    inception_cov is ignored is inception is provided.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noted, thx.

cat("\nStability Selection for PLNnetwork: ")
cat("\nsubsampling: ")

stabs_out <- future.apply::future_lapply(subsamples, function(subsample) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stability_selection() uses:

  • future::lapply() for PLNnetwork but
  • lapply() for ZIPLNnetwork
    Should we switch to lapply() everywhere ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used lapply in place of future_lapply for debugging (the later suppress the output which is not very convenient for debuging). For the moment, I suggest we use future_lapply for all parallel computing, and we will update this particular instance once we will have our complete rethinking of all parallel computing stuff.

#' @field nb_param number of parameters in the current PLN model
nb_param = function() {
res <- self$p * self$d + (sum(private$Omega != 0) - self$p)/2L +
res <- self$p * self$d + self$n_edges +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this formula, we don't count diagonal coefficients of $\Omega$. Although they are not (always) penalized, they should still be treated as parameters when computing e.g. BIC ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now applies to

nb_param_pln = function() {
as.integer(self$p * self$d + self$n_edges)
},

@jchiquet jchiquet marked this pull request as ready for review February 24, 2024 16:20
Copy link
Collaborator

@mahendra-mariadassou mahendra-mariadassou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good for me (and I should indeed have looked at it sooner...)

@jchiquet jchiquet merged commit 9c0b27f into master Mar 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants