stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.57k stars 368 forks source link

Feature/3176 json #3191

Closed mitzimorris closed 1 year ago

mitzimorris commented 1 year ago

Submission Checklist

Summary

Add callback writer to output JSON data.

Intended Effect

Allow structured outputs in JSON format, per design-docs spec https://github.com/stan-dev/design-docs/blob/master/designs/0032-stan-output-formats.md

How to Verify

Unit tests

Side Effects

None

Documentation

N/A

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): Coumbia University

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

mitzimorris commented 1 year ago

OK, ready for re-review

mitzimorris commented 1 year ago

looking at write_vector code - if a vector is empty, we should output the empty vector, no? current implementation doesn't.

mitzimorris commented 1 year ago

ready for re-re-review

bob-carpenter commented 1 year ago

The efficient way to do this if you know the instantiations at compile time would be with the CRTP. But I wouldn't do that if the virtual function calls aren't a bottleneck.

SteveBronder commented 1 year ago

Actually @mitzimorris and @WardBrian one last thing, how should NA and infinite values get handled here? I'm unfamiliar with how json should handle these.

WardBrian commented 1 year ago

We print them such that the JSON parser we use can read them back in. This involves using some non-standard atoms, since the JSON spec provides no proper format for these:

Value ""JSON""
Not-a-number NaN
Infinity Inf
Negative infinity -Inf
mitzimorris commented 1 year ago

OK to merge?

WardBrian commented 1 year ago

I think so, @SteveBronder?

SteveBronder commented 1 year ago

Yep good to merge!