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

Fit outcome model tests #149

Merged
merged 6 commits into from
Nov 8, 2023
Merged

Fit outcome model tests #149

merged 6 commits into from
Nov 8, 2023

Conversation

mvankessel-EMC
Copy link
Collaborator

I've added tests for each parameter for fitOutcomeModel().

There are a several things that I'd like to clarify, if the behavior is intended:

  1. interactionCovariateIds, excludeCovariateIds, and includeCovariateIds currently allows negative values.
  2. profileGrid and profileBounds are allowed to be NULL / NA simultaneously.
  3. profileBounds may be <= 0.
  4. prior and control get silently not used, when useCovariates is FALSE, without any message or warning.

@mvankessel-EMC mvankessel-EMC self-assigned this Oct 5, 2023
@schuemie
Copy link
Member

Great work! To answer your questions:

  1. interactionCovariateIds, excludeCovariateIds, and includeCovariateIds currently allows negative values.

In theory one could create negative covariate IDs. Currently nothing would break if you did.

  1. profileGrid and profileBounds are allowed to be NULL / NA simultaneously.

Yes. I sometimes use this to avoid generating a likelihood profile (computing a likelihood profile can be computationally expensive).

  1. profileBounds may be <= 0.

Yep. Bounds are on a log scale (as documented), so can be negative.

  1. prior and control get silently not used, when useCovariates is FALSE, without any message or warning.

Yes, we only allow shrinkage on covariates that are not the effect of interest. If there are no covariates that are not the effect of interest, you can specify prior and control but they have no effect. Not sure if we should message about that.

@mvankessel-EMC
Copy link
Collaborator Author

Ok, then feel free to merge.

@schuemie schuemie merged commit f953bad into develop Nov 8, 2023
10 checks passed
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.

None yet

2 participants