epinowcast / epidist

An R package for estimating epidemiological delay distributions
http://epidist.epinowcast.org/
Other
9 stars 4 forks source link

Add GitHub action to check model running using `cmdstanr` #104

Closed athowes closed 3 weeks ago

athowes commented 2 months ago

We could have a GitHub action to check that our Stan code works with the latest version of cmdstanr.

See #65.

This is what the action: https://github.com/epinowcast/epinowcast/blob/main/.github/workflows/check-cmdstan.yaml is doing inside of epinowcast.

To make this change we would need to copy over this, change the parts that are epinowcast specific. For sure this part:

https://github.com/epinowcast/epinowcast/blob/886b45cb4bc5f338fa53d22e83d25335c55b1a4a/.github/workflows/check-cmdstan.yaml#L60-L77

      - name: Compile model and check syntax
        run: |
          model <- epinowcast::enw_model()
          # If the model is not syntactically correct above will fail
          # however it may be correct enougth to compile but still contain
          # soft depreciated syntax and so we check the syntax again below 
          # and test the output.
          message <- capture.output(
            model$check_syntax(pedantic = FALSE),
            type = "message"
          )
          # We can't use TRUE here as pendatic check return lots of false
          # positives related to our use of functions.
          stopifnot(
            length(message) != 0 &&
            all(message == "Stan program is syntactically correct")
          )
        shell: Rscript {0}

Do we need to agree on which functions will be tested they can run? What about user input Stan functionality? Basically how many possible variations should we be checking?

Also I don't think we have a function to just compile, so we'd be also running. I imagine this would slow things down. Perhaps there is a way we can use fn to just compile and not run?

seabbs commented 2 months ago

We use make_stancode to get the standcode and pass that to cmdstanr to compile here I think.

I think we just want to check that our inserted stan makes a valid stan model using the base brms settings and trust brms to otherwise produce valid stan code.

athowes commented 1 month ago

I wonder how this is different to having a unit test which checks the code compiles, which we already have?

(Thinking of having a go at closing this issue now.)

seabbs commented 1 month ago

It can not fail but still have depreciation warnings essentially. Its proved useful to pull compiling out of unit tests as it makes clear what is failing.

athowes commented 1 month ago

By pull out do you mean to remove the compliation unit test? Or you think both serve useful purpose?

athowes commented 4 weeks ago

This is the code that needs to be replaced with something from epidist:

model <- epinowcast::enw_model()
message <- capture.output(model$check_syntax(pedantic = FALSE), type = "message")
# We can't use TRUE here as pendatic check return lots of false positives related to our use of functions.
stopifnot(length(message) != 0 && all(message == "Stan program is syntactically correct"))

It would look something like:

stancode <- epidist::epidist(data = ?, fn = brms::make_stancode)
mod <- cmdstan_model(stan_file = write_stan_file(stancode), compile = FALSE)
expect_true(mod$check_syntax())

The data here isn't doing anything, so I don't know what we can do to give it something so that it works (without actually generating plausible data). So long as it passes as_latent_individual I suppose.

seabbs commented 4 weeks ago

Yes - you can see the approach we took in the delays paper for generating stan code as a guide.