add design doc to align on MCMC options enhancements

[danielinteractive]

Hi @gowerc and team,
please find inside a design doc to start alignment on MCMC options improvements. Thoughts/comments/concerns?
Thank you,
Daniel

[github-actions]

✅ All contributors have signed the CLA
_{Posted by the CLA Assistant Lite bot.}

[danielinteractive]

I have read the CLA Document and I hereby sign the CLA

[gowerc]

Hey @danielinteractive - Sorry was just wondering what your planned timeline was for implementing this ? I ask just because we had a CRAN test failure and need to upload a new patch version before 2025-02-10. Was wondering if you wanted me to hold off for you to add this in? If not I'll just submit the new patch version now.

[danielinteractive]

Thanks @gowerc for asking, as soon as we have an aligned design in place (i.e. this PR merged) then I would code this asap, within a few days. On the other hand, we might need further adjustments downstream for handling the multiple chain output (not sure). But at least this part from this design doc we could fold into this CRAN update I think.

[gowerc]

@danielinteractive - Thanks for updating this, I just did another full review and added a few comments I think all of which are minor. Once they are resolved I am happy to merge

[danielinteractive]

Thanks @gowerc , all comments are addressed now, the only conversation remaining is about the seed, can you please have a look, thanks

[bailliem]

Re: the seed discussion, I think it makes more sense as it is now implemented using set.seed() explicitly. As @gowerc mentioned, it is more consistent across the various methods. @luwidmer , any concerns?

[danielinteractive]

Re: the seed discussion, I think it makes more sense as it is now implemented using set.seed() explicitly. As @gowerc mentioned, it is more consistent across the various methods. @luwidmer , any concerns?

Thanks @bailliem for chipping in! I think the current implementation with hardcoding the rstan seed is suboptimal, because it conflicts with the rest of the control function design, where you can override all arguments from the user side. I would like to highlight that also in the current design doc proposal the user can and should use set.seed() in their script/code, which will set the rstan seed as in the current implementation - in that sense it is consistent still with other rbmi methods. However, expert users will have the chance to set the seed argument for rstan explicitly if they need to - which aligns with the control function design.

[bailliem]

Re: the seed discussion, I think it makes more sense as it is now implemented using set.seed() explicitly. As @gowerc mentioned, it is more consistent across the various methods. @luwidmer , any concerns?

Thanks @bailliem for chipping in! I think the current implementation with hardcoding the rstan seed is suboptimal, because it conflicts with the rest of the control function design, where you can override all arguments from the user side. I would like to highlight that also in the current design doc proposal the user can and should use set.seed() in their script/code, which will set the rstan seed as in the current implementation - in that sense it is consistent still with other rbmi methods. However, expert users will have the chance to set the seed argument for rstan explicitly if they need to - which aligns with the control function design.

@danielinteractive , I agree. The UX experience and further control here makes sense. Happy with this proposal

[gowerc]

I'm honestly indifferent here (both sides have equal benefits and trade offs to me) so am happy to proceed with Daniels proposal if that is the desired direction

[danielinteractive]

Thanks @bailliem and @gowerc