Complex numbers vals_c #1032

nickdidio · 2021-08-12T18:48:14Z

Submisison Checklist

Run tests: ./runCmdStanTests.py src/test
Declare copyright holder and open-source license: see below

Summary:

Allow for intake of complex numbers in Stan. This PR mainly adds a new function called vals_c which allow for intake of complex numbers in a Stan program.

Documentation

All documentation for complex number is available at stan-dev/docs#378.

How to Verify:

Added more tests in /src/test/interface/io/json/json_data_test.cpp

Run tests: ./runCmdStanTests.py src/test

Side Effects:

None

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): Nicholas DiDio

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

Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

…mplex-numbers2

… into complex-numbers2

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-08-12T19:55:15Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.07	3.03	1.01	1.17% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.98	-2.24% slower
eight_schools/eight_schools.stan	0.11	0.11	1.05	4.31% faster
gp_regr/gp_regr.stan	0.16	0.16	0.99	-1.36% slower
irt_2pl/irt_2pl.stan	5.82	5.85	1.0	-0.37% slower
performance.compilation	89.44	87.15	1.03	2.56% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.78	8.92	0.98	-1.64% slower
pkpd/one_comp_mm_elim_abs.stan	30.16	29.96	1.01	0.67% faster
sir/sir.stan	130.05	132.82	0.98	-2.13% slower
gp_regr/gen_gp_data.stan	0.03	0.03	1.01	0.94% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.99	3.02	0.99	-1.08% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.41	0.39	1.04	3.77% faster
arK/arK.stan	1.92	1.87	1.03	2.66% faster
arma/arma.stan	0.83	0.83	1.0	-0.11% slower
garch/garch.stan	0.53	0.53	0.99	-0.74% slower
Mean result: 1.00472463605

Jenkins Console Log
Blue Ocean
Commit hash: cb2485d

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

stan-buildbot · 2021-08-12T20:17:10Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	2.98	2.95	1.01	1.03% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.95	-5.42% slower
eight_schools/eight_schools.stan	0.11	0.1	1.04	4.27% faster
gp_regr/gp_regr.stan	0.16	0.16	0.99	-0.76% slower
irt_2pl/irt_2pl.stan	5.8	5.84	0.99	-0.65% slower
performance.compilation	90.24	87.46	1.03	3.08% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.7	8.6	1.01	1.15% faster
pkpd/one_comp_mm_elim_abs.stan	29.5	30.9	0.95	-4.72% slower
sir/sir.stan	128.8	126.73	1.02	1.61% faster
gp_regr/gen_gp_data.stan	0.04	0.03	1.05	4.47% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.0	2.99	1.0	0.08% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.41	0.93	-7.21% slower
arK/arK.stan	1.87	1.86	1.01	0.85% faster
arma/arma.stan	0.82	0.83	0.99	-1.28% slower
garch/garch.stan	0.53	0.53	1.0	-0.26% slower
Mean result: 0.998532021841

Jenkins Console Log
Blue Ocean
Commit hash: 75d0f30

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

bob-carpenter

This looks great. The requested changes are all minor.

bob-carpenter · 2021-08-12T21:27:20Z

src/cmdstan/io/json/json_data.hpp

@@ -119,6 +120,43 @@ class json_data : public stan::io::var_context {
    return empty_vec_r_;
  }

+  /**
+   * Read out a vector of complex numbers from the specifed name.
+   * @param name Name of Variable.


This needs @return doc.

It should also have some light doc on the presumed input format or a pointer to where it's described. Otherwise, the code's too cryptic.

src/cmdstan/io/json/json_data.hpp

bob-carpenter · 2021-08-12T21:31:45Z

src/cmdstan/io/json/json_data.hpp

+        offset *= dim_r[i];
+      }
+      for (int i = 0; i < vec_c.size(); ++i) {
+        vec_c[i] = std::complex<double>{static_cast<double>(val_r[i]),


Why does val_r[i] need to be cast here? Isn't it already a double value? And even if it's not a double value, calling the std::complex<double> constructor should automatically cast it to double because of the template specification.

bob-carpenter · 2021-08-12T21:33:58Z

src/test/interface/io/json/json_data_test.cpp

+                      const std::vector<size_t> &expected_dims) {
+  EXPECT_EQ(true, (jdata.contains_r(name) || jdata.contains_i(name)));
+  std::vector<size_t> dims = jdata.dims_r(name);
+  if (dims.size() == 1) {


You don't need this first conditional. pop_back() will be equivalent to clear() if there is only a single element in the container. You generally don't want to branch for this kind of thing as branching is very inefficient when there's branch-point misprediction.

bob-carpenter · 2021-08-12T21:34:29Z

src/test/interface/io/json/json_data_test.cpp

+  for (size_t i = 0; i < dims.size(); i++)
+    EXPECT_EQ(expected_dims[i], dims[i]);
+  std::vector<std::complex<double>> vals = jdata.vals_c(name);
+  ;


Remove the stray semicolon. It's a valid empty statement, but it's not doing anything.

bob-carpenter · 2021-08-12T21:35:07Z

src/test/interface/io/json/json_data_test.cpp

@@ -144,6 +177,18 @@ TEST(ioJson, jsonData_real_array_1D) {
  test_real_var(jdata, txt, "foo", expected_vals, expected_dims);
 }

+TEST(ioJson, jsonData_complex_array_1D) {


[question]
Is there a test that we get the right error message if the variable is not found?

We test if the variable name exists in test_complex_var

EXPECT_EQ(true, (jdata.contains_r(name) || jdata.contains_i(name)));

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-08-13T15:04:02Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.07	3.04	1.01	1.05% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.0	0.27% faster
eight_schools/eight_schools.stan	0.1	0.11	0.99	-1.0% slower
gp_regr/gp_regr.stan	0.16	0.16	1.02	2.1% faster
irt_2pl/irt_2pl.stan	5.84	5.86	1.0	-0.33% slower
performance.compilation	89.24	87.5	1.02	1.96% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.63	8.89	0.97	-2.94% slower
pkpd/one_comp_mm_elim_abs.stan	29.84	30.8	0.97	-3.21% slower
sir/sir.stan	128.49	132.17	0.97	-2.87% slower
gp_regr/gen_gp_data.stan	0.04	0.04	0.99	-0.96% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.03	3.0	1.01	1.05% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.4	1.0	0.17% faster
arK/arK.stan	1.87	1.87	1.0	-0.03% slower
arma/arma.stan	0.82	0.83	0.99	-0.82% slower
garch/garch.stan	0.53	0.53	1.0	0.14% faster
Mean result: 0.996640263936

Jenkins Console Log
Blue Ocean
Commit hash: 18b9ff6

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

bob-carpenter

Looks good. Thanks for updating.

SteveBronder and others added 12 commits June 24, 2021 16:25

comples for complex numbers!

1155655

Prototype for complex scalars complete

de58444

fix indexing

25f2e3d

Merge branch 'develop' of https://github.com/stan-dev/cmdstan into co…

bf77119

…mplex-numbers2

Deleted temporary test files

40c83a4

Merge branch 'develop' of https://github.com/stan-dev/cmdstan into co…

5181197

…mplex-numbers2

update to develop

d595f21

add little doc to vals_c

af0434b

Added test files for complex

409b3b4

Merge branch 'complex-numbers2' of https://github.com/nickdidio/cmdstan…

aa50731

… into complex-numbers2

Updated stan submodule commit

9a61a1c

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

cb2485d

…4.1 (tags/RELEASE_600/final)

Added include for complex in json_data.hpp

75d0f30

nickdidio changed the title ~~[WIP] Complex numbers vals_c~~ Complex numbers vals_c Aug 12, 2021

bob-carpenter requested changes Aug 12, 2021

View reviewed changes

nickdidio and others added 2 commits August 13, 2021 10:11

Minor changes based on Bob's comments

75e1ce1

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

18b9ff6

…4.1 (tags/RELEASE_600/final)

bob-carpenter approved these changes Aug 13, 2021

View reviewed changes

bob-carpenter merged commit 120cb36 into stan-dev:develop Aug 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Complex numbers vals_c #1032

Complex numbers vals_c #1032

nickdidio commented Aug 12, 2021 •

edited

Loading

stan-buildbot commented Aug 12, 2021

stan-buildbot commented Aug 12, 2021

bob-carpenter left a comment

bob-carpenter Aug 12, 2021

bob-carpenter Aug 12, 2021

bob-carpenter Aug 12, 2021

bob-carpenter Aug 12, 2021

bob-carpenter Aug 12, 2021

nickdidio Aug 13, 2021

stan-buildbot commented Aug 13, 2021

bob-carpenter left a comment

Complex numbers vals_c #1032

Complex numbers vals_c #1032

Conversation

nickdidio commented Aug 12, 2021 • edited Loading

Submisison Checklist

Summary:

Documentation

How to Verify:

Side Effects:

Copyright and Licensing

stan-buildbot commented Aug 12, 2021

stan-buildbot commented Aug 12, 2021

bob-carpenter left a comment

Choose a reason for hiding this comment

bob-carpenter Aug 12, 2021

Choose a reason for hiding this comment

bob-carpenter Aug 12, 2021

Choose a reason for hiding this comment

bob-carpenter Aug 12, 2021

Choose a reason for hiding this comment

bob-carpenter Aug 12, 2021

Choose a reason for hiding this comment

bob-carpenter Aug 12, 2021

Choose a reason for hiding this comment

nickdidio Aug 13, 2021

Choose a reason for hiding this comment

stan-buildbot commented Aug 13, 2021

bob-carpenter left a comment

Choose a reason for hiding this comment

nickdidio commented Aug 12, 2021 •

edited

Loading