openpharma / crmPack

Object-Oriented Implementation of CRM Designs
https://openpharma.github.io/crmPack/
19 stars 9 forks source link

Proposal: Reworking interface for providing JAGS code #550

Open dgkf opened 1 year ago

dgkf commented 1 year ago

Summary

Carrying forward some of the discussion started in #546, I wanted to kick off a proposal for a new mechanism for specifying JAGS code.

There is one pressing problem with the current solution, and one plausible problem for how well it generalizes.

  1. we encounter really esoteric issues when running covr due to the reliance on NSE and the way that covr modifies package code. We have a workaround for now, but avoiding it entirely would be preferred.
  2. the current mechanism can't accommodate all JAGS syntax, since not all JAGS code is syntactically valid R code. This doesn't appear to be an issue so far, but presumably there are models that users might want to implement that would run into errors. One specific example is the truncation syntax (dnorm(x, y)T(lower, [upper])).

I propose that crmPack provides a JAGS function as an interface to specify JAGS code. It can accept a string or an R expression through NSE, processing expressions similar to how we do it now to produce the string that gets written to the JAGS model file.

This would allow us to use the generalized string-as-code internally in the package to solve (1), while giving a more R-like interface for users that aren't running up against syntactic edge cases with the option to fall back to the string-as-code to solve (2).

Additional Information

Probably the biggest question surrounding this change would be the lifecycle management. To throw out one path as a proposal, I'd consider:

  1. Switching entirely to using this proposed string style internally
  2. Continue to accept functions from users of the package
  3. Introduce a deprecation warning, indicating that the function interface will be removed in the next major version and suggest changing the syntax from function() <code> to JAGS(<code>).
PuzzledFace commented 1 year ago

@dgkf The truncation issue - and other inconsistencies between R and JAGS syntax - can be handled by the (little-known) %_% operator, which is documented in the roxygen2 comments for h_jags_write_model in helpers_jags.R. This allows truncation to be specifed by, say, dnorm(x, y) %_% T(lower, ) in R code.

That said, I am sympathetic to the idea of moving from JAGS-as-R to JAGS-as-string, and considered suggesting exactly the same thing myself. It was whilst researching the justification for this that I discovered %_%, which resolved all my known difficulties with the current situation.

I would be interested in hearing from longer-standing members of the development team why the current solution was adopted.

dgkf commented 1 year ago

can be handled by the (little-known) %_% operator

Very cool. I didn't catch this in the code. Though I think the need to skirt JAGS-native code by shoving in custom infix operators underlines the crux of the issue here.

I'm of mixed opinions about the best style here. On the one hand, I appreciate how the R syntax gives us some nice quality-of-life improvements like leveraging R syntax highlighting and parenthesis matching to help reduce code typos. On the other hand, the R code is a bit misleading since it's not actually evaluable - instead users need to be cognizant of how the crmPack DSL translates to JAGS concepts.

I think both styles have merits, and weighing the benefits of a refactor is probably a call for developers and more frequent users of the package.