Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/reduce mem pathfinder #3325

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
87be301
Pathfinder lazily evaluates and writes parameters instead of taking e…
SteveBronder Dec 12, 2024
3c932f0
add docs
SteveBronder Dec 13, 2024
f1a8e56
remove stringstream from unique stream writer
SteveBronder Dec 13, 2024
48bbb6e
update laplace
SteveBronder Dec 13, 2024
0b46b4a
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 13, 2024
c644df0
remove newline
SteveBronder Dec 13, 2024
814f8df
fixing pathfinder return results
SteveBronder Dec 17, 2024
e484fa1
adds concurrent queue so writes are thread safe to one file
SteveBronder Dec 18, 2024
e88ca19
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 18, 2024
6b24bbf
fix docs for multi_stream_writer
SteveBronder Dec 18, 2024
0e48078
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 18, 2024
298761e
fix docs for multi_stream_writer
SteveBronder Dec 18, 2024
ae3740c
fix docs for multi_stream_writer
SteveBronder Dec 18, 2024
b98f60f
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 18, 2024
a18d7e9
update normal_glm test for better inits
SteveBronder Dec 18, 2024
9250e76
Merge remote-tracking branch 'refs/remotes/origin/feature/reduce-mem-…
SteveBronder Dec 18, 2024
4a058bc
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 18, 2024
09d66e5
update headers
SteveBronder Dec 18, 2024
c25ec46
[Jenkins] auto-formatting by clang-format version 10.0.0-4ubuntu1
stan-buildbot Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 149 additions & 1 deletion src/stan/callbacks/unique_stream_writer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ class unique_stream_writer final : public writer {
return;
write_vector(names);
}
/**
* Writes a set of names on a single line in csv format followed
* by a newline.
*
* Note: the names are not escaped.
*
* @param[in] names Names in a std::vector
* @param[in, out] ss A stringstream to use as a buffer
*/
void operator()(const std::vector<std::string>& names, std::stringstream& ss) {
if (output_ == nullptr)
return;
write_vector(names, ss);
}

/**
* Get the underlying stream
Expand All @@ -77,6 +91,20 @@ class unique_stream_writer final : public writer {
return;
write_vector(values);
}
/**
* Writes a set of values in csv format followed by a newline.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values Values in a std::vector
* @param[in, out] ss ignored
*/
void operator()(const std::vector<double>& values, std::stringstream& ss) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I like these overloads --

  1. They're not in the base class writer
  2. Is there a reason we cant just have the buffer be a private member in the class or something similar?
  3. The "Note" in the doc comment is wrong, since we make no attempt to make sure the settings of the stringstream are the same as the ostream. I think, among other things, this would break the way cmdstan sets the number of digits?

Separately, isn't std::ostream buffered internally anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we cant just have the buffer be a private member in the class or something similar?

I didn't want to introduce state directly in the class

The "Note" in the doc comment is wrong, since we make no attempt to make sure the settings of the stringstream are the same as the ostream. I think, among other things, this would break the way cmdstan sets the number of digits?

I totally missed this. Yeah my through was that having a little buffer we reuse would save some memory, but idt it's worth the overhead of config etc. And we only use it for the case of a std vector really

if (output_ == nullptr)
return;
write_vector(values);
}

/**
* Writes multiple rows and columns of values in csv format.
Expand All @@ -88,12 +116,96 @@ class unique_stream_writer final : public writer {
* parameters in the rows and samples in the columns. The matrix is then
* transposed for the output.
*/
void operator()(const Eigen::Ref<Eigen::Matrix<double, -1, -1>>& values) {
void operator()(const Eigen::Matrix<double, -1, -1>& values) {
if (output_ == nullptr)
return;
*output_ << values.transpose().format(CommaInitFmt);
}

/**
* Writes multiple rows and columns of values in csv format.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values A matrix of values. The input is expected to have
* parameters in the rows and samples in the columns. The matrix is then
* transposed for the output.
* @param[in, out] ss ignored
*/
void operator()(const Eigen::Matrix<double, -1, -1>& values, std::stringstream& /* ss */) {
if (output_ == nullptr)
return;
*output_ << values.transpose().format(CommaInitFmt);
}

/**
* Writes a row of values in csv format.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values A row vector of values. The input is expected to have
* samples in the columns.
* @param[in, out] ss ignored
*/
void operator()(const Eigen::Matrix<double, 1, -1>& values) {
if (output_ == nullptr)
return;
*output_ << values.format(CommaInitFmt);
}

/**
* Writes a row of values in csv format.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values A row vector of values. The input is expected to have
* samples in the columns.
* @param[in, out] ss ignored
*/
void operator()(const Eigen::Matrix<double, 1, -1>& values, std::stringstream& /* ss */) {
if (output_ == nullptr)
return;
*output_ << values.format(CommaInitFmt);
}

/**
* Writes a row of values in csv format.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values A column vector of values. The input is expected to have
* samples in the rows. The matrix is then
* transposed for the output.
* @param[in, out] ss ignored
*/
void operator()(const Eigen::Matrix<double, -1, 1>& values) {
if (output_ == nullptr)
return;
*output_ << values.transpose().format(CommaInitFmt);
}

/**
* Writes a row of values in csv format.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] values A column vector of values. The input is expected to have
* samples in the rows. The matrix is then
* transposed for the output.
*/
void operator()(const Eigen::Matrix<double, -1, 1>& values, std::stringstream& /* ss */) {
if (output_ == nullptr)
return;
*output_ << values.transpose().format(CommaInitFmt);
}



/**
* Writes the comment_prefix to the stream followed by a newline.
*/
Expand All @@ -114,6 +226,18 @@ class unique_stream_writer final : public writer {
*output_ << comment_prefix_ << message << std::endl;
}

/**
* Writes the comment_prefix then the message followed by a newline.
*
* @param[in] message A string
@ @param[in, out] ss ignored
*/
void operator()(const std::string& message, std::stringstream& /* ss */) {
if (output_ == nullptr)
return;
*output_ << comment_prefix_ << message << std::endl;
}

private:
/**
* Comma formatter for writing Eigen matrices
Expand Down Expand Up @@ -153,6 +277,30 @@ class unique_stream_writer final : public writer {
}
*output_ << v.back() << std::endl;
}
/**
* Writes a set of values in csv format followed by a newline.
*
* Note: the precision of the output is determined by the settings
* of the stream on construction.
*
* @param[in] v Values in a std::vector
* @param[in, out] ss A stringstream to use as a buffer
*/
template <class T>
void write_vector(const std::vector<T>& v, std::stringstream& ss) {
if (output_ == nullptr)
return;
if (v.empty()) {
return;
}
auto last = v.end();
--last;
for (auto it = v.begin(); it != last; ++it) {
ss << *it << ",";
}
ss << v.back() << std::endl;
*output_ << ss.str();
}
};

} // namespace callbacks
Expand Down
4 changes: 2 additions & 2 deletions src/stan/services/optimize/laplace_sample.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ void laplace_sample(const Model& model, const Eigen::VectorXd& theta_hat,
// create log density functor for vars and vals
std::stringstream log_density_msgs;
auto log_density_fun
= [&](const Eigen::Matrix<stan::math::var, -1, 1>& theta) {
= [&](auto& theta) {
return model.template log_prob<true, jacobian, stan::math::var>(
const_cast<Eigen::Matrix<stan::math::var, -1, 1>&>(theta),
theta,
&log_density_msgs);
};

Expand Down
Loading