Skip to content

Comments

Suggestions for VPC simulation#22

Merged
roninsightrx merged 2 commits intovpcfrom
vpc-suggestions
Sep 30, 2025
Merged

Suggestions for VPC simulation#22
roninsightrx merged 2 commits intovpcfrom
vpc-suggestions

Conversation

@mccarthy-m-g
Copy link
Collaborator

There's currently a lot of code duplication in #12. Specifically, run_vpc() is basically a copy of run_eval().

Instead, this PR integrates VPC data simulation into run_eval(), which can be controlled via a new .vpc_options argument. By default, we will simulate the VPC data in run_eval(), but it can optionally be skipped. There's also an option to skip MAP bayesian estimates (this was mainly to speed up the VPC tests).

I also added a VPC plot method + tests.

@mccarthy-m-g mccarthy-m-g added the enhancement New feature or request label Sep 29, 2025
# Conflicts:
#	R/run_vpc.R
#	man/run_vpc.Rd
@roninsightrx
Copy link
Contributor

Agree, it's better to keep the main code only in run_eval() and not repeat in separate function. Since sims are relatively fast compared to prospective runs, we can indeed keep them switched on so we can compute VPC and NPDE (coming in #14 ) so we can show even more stats by default.

Copy link
Contributor

@roninsightrx roninsightrx left a comment

Choose a reason for hiding this comment

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

lgtm!

plot_fun(x, ...)
}

plot_vpc <- function(res, ...) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok with leaving this in, we can see how useful it is. But often creating a VPC is somewhat of an iterative process: sometimes it looks better with the observations plotted, sometimes without. Sometimes better on log-scale, sometimes on normal scale etc. So I almost never am satisfied with the default vpc() call, need to adjust with arguments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have the ... there so you can adjust with arguments. The help page links to vpc() so it's easy to see what those arguments are:

Screenshot 2025-09-30 at 9 31 03 AM

@roninsightrx roninsightrx merged commit 70406b5 into vpc Sep 30, 2025
@roninsightrx roninsightrx deleted the vpc-suggestions branch September 30, 2025 16:19
roninsightrx added a commit that referenced this pull request Sep 30, 2025
* add more stats

* add tests + update docs

* fix tests

* minor build fixes

* initial implementation of vpc

* update docs + add tests

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* Apply suggestion from @mccarthy-m-g

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>

* move to S3 + print function + few more stats

* fix tests

* Update R/run_vpc.R

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update R/run_vpc.R

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* update docws

* integrate VPC data simulation into `run_eval()`, add VPC plot method + tests (#22)

* fix progress bars

* fix + workaround for issues with progressr

* more fixes

* minor updates progress messages

* update docs

---------

Co-authored-by: Michael McCarthy <51542091+mccarthy-m-g@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants