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

added maxDimCovHistory control option for RW_block sampler #1329

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

danielturek
Copy link
Member

The title says it all.

@danielturek
Copy link
Member Author

I'll leave this to (hopefully) get merged at some point in the future.

@paciorek
Copy link
Contributor

In email I suggested we might want to instead have the option be MCMCsaveHistoryCov, allowing a user to separately say whether they want the covariance saved in addition to the other history stuff.

Changing the max dimension seems a bit more round-about to me than just having the user directly specify whether they want the covariance saved?

That said, as I think about it more, a user could well want to save the covariance only for some of the nodes, so keeping the option as something that relates to dimension makes sense. Otherwise they wouldn't be able to say they want covariance saved only for the lower-dimensional nodes...

@danielturek
Copy link
Member Author

@paciorek I just saw your comment here. Thanks for your thoughts on this.

@paciorek
Copy link
Contributor

Sorry to confuse things - there's (1) the question of whether the user should set the max dimension or simply say whether the covariances should be saved and (2) the question of system option vs. sampler control option. I hadn't noticed that the proposal was to use a sampler control option.

On (1), I've come around to the idea of the user controlling the max dimension.

On (2), as far as system option vs. sampler control option, I'm fine with having it as a sampler option.

@paciorek paciorek merged commit ae54aaf into devel Dec 15, 2023
5 of 8 checks passed
@paciorek paciorek deleted the RWblock_max_dim branch December 15, 2023 03:39
@paciorek
Copy link
Contributor

paciorek commented Dec 17, 2023

Hmm, @danielturek it looks like you changed line 1556 to

scale <- extractControlElement(control, 'scale',               1)

but then originalScale is not defined. I'm changing that line (on devel) to be

scaleOriginal <-  extractControlElement(control, 'scale',               1)

but let me know if I'm missing something.

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