Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1174 +/- ##
========================================
Coverage 91.23% 91.23%
========================================
Files 15 15
Lines 6070 6173 +103
========================================
+ Hits 5538 5632 +94
- Misses 532 541 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks Aki! This one might take me some time to review. |
There was a problem hiding this comment.
I haven't reviewed the code yet but the failure on WSL is because
Error: Additional model methods are not currently available with WSL CmdStan and will not be compiled
Adding skip_if(os_is_wsl()) to those specific tests should fix it.
EDIT: oops I meant to "request changes" not "approve" yet
There was a problem hiding this comment.
I haven't reviewed the code yet but the failure on WSL is because a few of them error with
Error: Additional model methods are not currently available with WSL CmdStan and will not be compiled
Adding skip_if(os_is_wsl()) to those specific tests should fix it.
|
Just to add a note, it would be nice if some of these IO utilities were moved somewhere independent, so we could use them from e.g. bridgestan or similar (I know that having more packages is more CRAN pain...) |
|
I still haven't had a chance to go through this myself yet (I definitely will at some point), but I asked codex to take a look to see if it could find anything that's clearly an issue:
I haven't verified yet that these are all actually issues. But if 1 is true then I guess we could break backwards compatibility here because we're going to do v1.0. We could say that any list with names will be interpreted as a tuple. It's not ideal, but it's doable if there's not another solution. |
Closes #925
Stan examples by me, @spinkney and @WardBrian (from https://discourse.mc-stan.org/t/proof-of-concept-binary-output-format-for-cmdstan/40846/67). Careful iterative prompting by me. Code , tests and PR description assisted by Claude.
Fix tuple and complex variable handling
Summary
CmdStan 2.38+ introduced tuple and complex types, which use new naming
conventions in CSV output that CmdStanR did not understand. This PR adds
full support for these types across metadata parsing, model methods, and
init value handling.
repair_variable_names()andvariable_dims()to handle complexsuffixes (
.real/.imag) and tuple separators (:) in CSV column namesvariable_skeleton()andunconstrain_draws()for models withtuple and complex parameters
mod$sample(init = fit)now correctly passestuple parameter values as nested JSON objects
write_stan_json()support for tuple values as named listsProblem
CmdStan CSV headers now use three different separators:
.between digitsbeta.1.2→beta[1,2]:b_tuple:1:1.2→b_tuple:1:1[2].real/.imagz.real→z[real]These can combine:
arr_pair.1:1(array + tuple),z3D.1.1.1.real(array + complex),
nested:2:2.real(tuple + complex).Before this PR, the following issues occurred:
Warning:
NAs introduced by coercionfromvariable_dims()onevery model with complex or tuple types —
as.numeric("imag")returnsNA.
Wrong metadata:
fit$metadata()$stan_variable_sizescontainedNAfor complex variables and incorrect dimensions.
Broken
variable_skeleton(): ReturnedNAnames for tupleparameters because
create_skeleton()couldn't match Stan-level names(
"b_tuple") against C++ leaf names ("b_tuple.1.1").Crashing
unconstrain_draws(): Tuple parameters were silentlydropped from the draw subset (Stan-level name
"b_tuple"not found inleaf names
"b_tuple:1:1"), causing CmdStan to receive the wrong numberof scalars.
Missing init values:
mod$sample(init = fit)dropped tupleparameters entirely, and
write_stan_json()could not serialize tuplevalues (heterogeneous lists crashed
list_to_array()).Changes
R/csv.Rrepair_variable_names()— Detects and strips.real/.imagsuffixesbefore the dot-to-bracket conversion, then re-attaches them in the correct
position. The
:tuple separator passes through unchanged.variable_dims()— For non-numeric indices ("real","imag", tupleindices like
"1:2"), counts unique values across all entries for thatdimension position instead of calling
as.numeric().R/utils.RNew helper functions for bridging between Stan-level names and leaf names:
stan_param_has_leaf()— Checks if Stan-level names have matchingleaf names using
":"prefix matching.expand_stan_params_to_leaves()— Expands Stan-level names totheir leaf equivalents for
posterior::subset_draws().is_tuple_type()— Detects tuple parameters frommodel_variablestype info (
$typeis a list for tuples).build_tuple_init_value()— Recursively reconstructs a nestednamed-list init value from flat leaf draws, also validating for NA/Inf.
.extract_draw_value()— Shared helper for the draw extractionpipeline.
create_skeleton()— Expands tuple Stan-level names to leaf componentsfrom
param_metadata_using"."prefix matching.R/fit.Runconstrain_draws()andunconstrain_variables()— Usestan_param_has_leaf()for the zero-length parameter check andexpand_stan_params_to_leaves()for draw subsetting.R/args.Rvalidate_fit_init()— Usesstan_param_has_leaf()instead of%in%.process_init.draws()— Separates parameters into tuple and non-tuple.For tuples, uses
build_tuple_init_value()to reconstruct nested initvalues. NA/Inf validation is done inline during extraction.
R/data.Rwrite_stan_json()— Detects tuple-style named lists (keys"1","2", ...) and processes them recursively instead of callinglist_to_array().New helpers:
is_tuple_list(),prepare_tuple_for_json().Tests
tests/testthat/resources/stan/tuple_complex.stan— Test model withtuple and complex types as both parameters and generated quantities:
nested tuples, tuples with complex, arrays of tuples, complex vectors,
complex matrices, arrays of complex matrices.
tests/testthat/test-tuple-complex.R— 137 tests covering:repair_variable_names,unrepair_variable_names,variable_dimswith all tuple/complex name patternsstan_param_has_leaf,expand_stan_params_to_leaves)write_stan_jsonwith tuple valuesbuild_tuple_init_valuereconstructionvariable_skeleton(),unconstrain_draws(),init = fitround-trip, manual init listsTest plan
test-csv.Rtests passtest-data.Rtests passtest-fit-init.Rtests passtest-tuple-complex.Rtests passparams, complex scalar/vector/matrix/array params, simple/nested/arrayed
tuples, tuples as parameters and generated quantities
Copyright and Licensing
Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Aki Vehtari
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: