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

This is a transference of the ergm-private" "control_llik" branch to the public version ergm 4.0. #337

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

handcock
Copy link
Contributor

@handcock handcock commented Jul 1, 2021

The branch consolidates control arguments to the likelihood functions used in ergm.estimate into a single list variable called "control.llik".
This simplifies the calls to the functions.

The branch also adds a simple hook in "ergm.estimate" for the Kpenalty likelihood functions. These are required for the "ergm'tapered" package to interoperate with "ergm".

Also fixed a few documentation typos in "ergm".

…the public version ergm 4.0.

The branch consolidates control arguments to the likelihood functions used in ergm.estimate into a single list variable called "control.llik".
This simplifies the calls to the functions.

The branch also adds a simple hook in "ergm.estimate" for the Kpenalty likelihood functions. These are required for the "ergm'tapered" package to interoperate with "ergm".

Also fixed a few documentation typos in "ergm".
@handcock
Copy link
Contributor Author

handcock commented Jul 1, 2021

To make life easy, I created a branch of ergm 4.0 and manually made the edits from the private "control_llik" branch. As it turned out it was necessary as ergm 4.0 had evolved so that automatic merging would have been a mess.

@krivit
Copy link
Member

krivit commented Jul 1, 2021

Thanks, Mark! I'll take a look the week after Sunbelt.

These required extensive algebra and computation. However, they dramatically improve the computational speed and accuracy of the Kpenalty method.
@@ -48,6 +48,7 @@ Suggests:
tergm (>= 4.0.0),
ergm.count (>= 4.0),
ergm.userterms (>= 3.10.0),
ergm.tapered (>= 1.0.0),
Copy link
Member

Choose a reason for hiding this comment

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

Since ergm.tapered is not on CRAN, we also need to add a Remotes: directive.

That said, we might want to keep it away from requirements to avoid a chicken-and-egg problem the first time we release.

@@ -30,36 +30,69 @@
#' \eqn{q}-vectors of estimating function values.
#' @keywords internal
#' @export
ergm.estfun <- function(stats, theta, model, ...){
ergm.estfun <- function(stats, theta, model, exclude=NULL, ...){
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure whether this is a good idea in its current form:

  • It doesn't seem to do anything that subsetting the function's output wouldn't already. Both matrices and mcmc.list objects already support column subsetting. If it did the subsetting prior to the estimating function calculation, that might save computing time, but it doesn't do that, as far as I can tell.
  • Matching columns by name can be very unreliable, because in ergm you can have duplicate coefficient names, at least at the moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally replying to this. I tried the subsetting but can not get it to work as there are also places like ergm.bridge where it is needed. In the end I just left it as an option as it is cleaner and more flexible.

@@ -57,6 +57,12 @@ ergm.stocapprox <- function(init, nw, model,
stats <- model$nw.stats - NVL(model$target.stats,model$nw.stats)
target.stats <- model$target.stats
})

control.llik <- list(MCMLE.metric=control$MCMLE.metric, MCMLE.trustregion=control$SA.trustregion,
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to implement a control.ergm.llik.<METRIC>() family of functions so that the user could potentially specify different parameters for each metric and get argument completion etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe latter. At the moment the list is small and has not changed for a long while (until ergm.tapered/Kpenalty. At some point it may be worth adding if/when the list grows

Comment on lines 556 to +558
MCMLE.metric=c("lognormal", "logtaylor",
"Median.Likelihood",
"EF.Likelihood", "naive"),
"EF.Likelihood", "Kpenalty", "naive"),
Copy link
Member

Choose a reason for hiding this comment

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

Point of clarification: is Kpenalty an alternative way to estimate the likelihood ratio, or is it estimating the penalised likelihood?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kpenalty estimates the penalized likelihood. Explicitly, is computed the log-likelihood ratio (using lognormal) and then the kurtosis estimates to add to the log-likelihood ratio as the penalty. Otherwise, it is close to the standard lognormal version. The gradient is computed (mostly) exactly.

Added a MPLE.save.xmat control argument to return the design matrix from the MPLE fit (if any). The current and retained default is to remove it.
control$MCMC.esteq.exclude.statistics allows network statistics to not be included in the estimating equations. There equations determine most convergence diagnostics. For some models there are some terms that are not zero at convergence and should be removed.  For example, this supports tapering models which may have terms with non-specific means. The estimating equations are used in a number of places and the code follows those places.

Specifically, it is a vector of  strings being the names of the statistics to be excluded from
the estimating equations.
It now passes R CMD check
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