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

[base_hmc] add 'const noexcept' to stepsize accessors #3124

Closed kedartal closed 2 years ago

kedartal commented 2 years ago

Submission Checklist

Summary

Add const noexcept modifiers to the three accessors related to step-size (nominal, current, and jitter).

Intended Effect

Allow retrieving those values from a const object.

How to Verify

Trivial.

Side Effects

None.

Documentation

N/A.

Copyright and Licensing

Tal Kedar

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

kedartal commented 2 years ago

The second test in .../src/test/unit/services/util/gq_writer_test.cpp fails regardless of this PR

[ RUN      ] ServicesUtilGQWriter.t2
src/test/unit/services/util/gq_writer_test.cpp:44: Failure
Expected equality of these values:
  count_matches(",", sample_ss.str())
    Which is: 5
  2
[  FAILED  ] ServicesUtilGQWriter.t2 (0 ms)
TEST_F(ServicesUtilGQWriter, t2) {
  stan::callbacks::stream_writer sample_writer(sample_ss, "");
  stan::callbacks::stream_logger logger(logger_ss, logger_ss, logger_ss,
                                        logger_ss, logger_ss);
  boost::ecuyer1988 rng1 = stan::services::util::create_rng(0, 1);
  std::vector<double> draw;
  draw.push_back(-2.345);
  draw.push_back(-6.789);
  stan::services::util::gq_writer writer(sample_writer, logger, 2);
  writer.write_gq_values(model, rng1, draw);
  // model test_gq.stan generates 3 values, 2 commas
  EXPECT_EQ(count_matches(",", sample_ss.str()), 2);
}
bob-carpenter commented 2 years ago

Hi, @kedartal. Could you please clarify whether you or your company own the copyright to the contribution. Thanks.

We need to get tests passing before we can merge. I agree that your PR isn't the culprit here---at most it would change what compiles and what doesn't. I'm guessing the failure is due to Jenkins' state, but @serban-nicusor-toptal should know.

rok-cesnovar commented 2 years ago

The issue with the test is that a recent stanc3 PR broke develop - will be addressed once https://github.com/stan-dev/stanc3/pull/1193 is merged. Will restart the tests once it is. Sorry about that.

kedartal commented 2 years ago

Hi, @kedartal. Could you please clarify whether you or your company own the copyright to the contribution. Thanks.

@bob-carpenter Done, not that there's much difference yet :)

bob-carpenter commented 2 years ago

Thanks!