Skip to content

[WIP] Adding complex scalars and arrays#2

Open
SteveBronder wants to merge 32 commits into
rybern:constraint-refactor-2from
SteveBronder:feature/complex-types-proto
Open

[WIP] Adding complex scalars and arrays#2
SteveBronder wants to merge 32 commits into
rybern:constraint-refactor-2from
SteveBronder:feature/complex-types-proto

Conversation

@SteveBronder
Copy link
Copy Markdown

This is a proto of complex types in Stan! This PR just focuses on complex scalars/arrays and we can do complex vectors and matrices in a seperate PR. I built it on top of @rybern 's constraint PR since I need the deserializer for these to work.

There's a couple issues in the generated Stan code I'll go over and one parser issue that's weird and I can't figure out.

For the mother_complex.stan test file there is one function that won't compile

  complex[] foo_3(complex t, int n) {
    return rep_array(t,n);
  }

Uncommenting that line gives back an error after parsing the AST of

Semantic error in './test/integration/good/code-gen/mother_complex.stan', line 95, column 2 to line 97, column 3:
   -------------------------------------------------
    93:    }
    94:  
    95:    complex[] foo_3(complex t, int n) {
           ^
    96:      return rep_array(t,n);
    97:    }
   -------------------------------------------------

Branches of function definition need to have the same return type. Instead, found return types array[] complex and array[] int

Which seems really odd? In the signatures test it looks like it is generating the correct signature for rep_array(complex, int)

rep_array(complex, int) => array[] complex

So I'm not sure where that array[] int is coming from? It has to be somewhere in the AST/parser/lexer. I've tried looking bottom up starting at the semantic checks but I can't figure out where it thinks it's getting an array of integers from.

For the generated code there's a couple small issues.

  1. I think I need a version of fill() for complex types so that it knows to fill up the real and imaginary part with the value we give.

  2. For code generated in data that get it's values from the context I think we need a vals_c for returning an std::vector. So in the below vals_r is going to return an std::vector<double> that can't be directly assigned.

    d_complex_1d_ar = std::vector<std::complex<double>>(N, std::numeric_limits<double>::quiet_NaN());
    d_complex_1d_ar = context__.vals_r("d_complex_1d_ar");

But if the context had a vals_c it can return back an std::vector<complex<double> and that should work for arrays as well.

  1. For things like unconstrained_param_names should an complex type return one name as p_complex or two names for p_complex.real, p_complex.imag?

  2. @BobCarpenter should all of the functions in prim for scalars and arrays work with complex? I know we don't want _lpdf etc. functions to accept complex types though I think I remember pretty much everything in prim should work with complex types. It might be good to write up a tech spec if you have the time

  3. @rokcesnovar did we ever figure out a name for accessing the real and imaginary parts of a complex type? I'm pro re and imag or .re() and .imag(). Anything really. What if we just made them indexable so users could do

complex a;
real b = a[1]; // get real part
real c = a[2]; // get imaginary part

Then under the hood we can just call .real() for [1] and .imag() for [2]. Though idk if that could end up being confusing for users or not.

Release notes

Adds complex scalars and arrays to the language

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause): Steve Bronder

SteveBronder and others added 29 commits March 26, 2021 14:26
Call new deserializer backend for constrained reads
… decls, fix error message and removed quiet_NaN for int
SteveBronder pushed a commit that referenced this pull request Apr 13, 2021
Switch to Fmt; move exit 0 to driver from lib module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants