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

likelihood warnings added to validate_task #53

Closed
wants to merge 1 commit into from

Conversation

alexnakagawa
Copy link

@alexnakagawa alexnakagawa commented Dec 31, 2019

Hi Jeremy,

As per #42 , I've added some code to the validate_task function in Likelihood.R that will do the following:

  1. Find all of the variable types in a factor_list
  2. check for whether there exists a "continuous" variable for a "density" outcome.
  3. If both are true, then a warning() is thrown.

Please let me know if you'd like me to write tests, as well as update the reference website with the added warning.

@jeremyrcoyle jeremyrcoyle changed the base branch from master to devel January 15, 2020 22:53
@jeremyrcoyle
Copy link
Collaborator

Hi Alex,

It's more complicated than checking if there is a combination of density and mean type nodes in the same likelihood. That combination is not a problem.

The issue is using bounding with density type nodes. See the example in the issue. The user defines an NPSEM with a Y node that has variable type variable_type('continuous', bounds = bounds) (this is a a bounded continuous variable). He also defines a likelihood with a corresponding Y node that is of the density type: define_lf(LF_fit, "Y", lrnr_glm) (the default type is "density"). This combination doesn't work. So, what's needed is to check for nodes where the likelihood is density type and the npsem variable type is bounded continuous.

This check can be added to the validate_task function defined here: https://github.com/tlverse/tmle3/blob/master/R/Likelihood.R#L53, as that function has access to both the likelihood factor list (self$factor_list) and the corresponding npsem (via tmle_task$npsem).

@alexnakagawa
Copy link
Author

Thanks for the clarification! Would it be sufficient to check for whether the "Y" node is of variable_type$type == "continuous? Or would there be cases where we'd have to check the "A" node?

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