Fix integrate_1d to use msgs ostream pointer like all other functions#1409
Conversation
|
Anyone ever seen cc1plus run out of memory on the Windows Stan unit tests? Seems pretty unrelated to the code in this PR... @serban-nicusor-toptal ? |
|
I think I've seen that before, it's sort of sporadic. I just kicked it off again so we will see |
|
I think we should go the other way and refactor toward passing references. I don't think our I/O code is robust enough to handle null pointers---there would have to be conditionals everywhere.
… On Oct 19, 2019, at 11:27 AM, seantalts ***@***.***> wrote:
Summary
Minor refactor of integrate_1d to use std::ostream* instead of std::ostream& like all other functions. Likely an oversight from its initial implementation.
Checklist
• Math issue #933
• Copyright holder: (fill in copyright holder information)
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses: Columbia University
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
• the basic tests are passing
• unit tests pass (to run, use: ./runTests.py test/unit)
• header checks pass, (make test-headers)
• docs build, (make doxygen)
• code passes the built in C++ standards checks (make cpplint)
• the code is written in idiomatic C++ and changes are documented in the doxygen
You can view, comment on, or merge this pull request online at:
#1409
Commit Summary
• Fix integrate_1d to use msgs ostream pointer like all other functions
File Changes
• M stan/math/prim/arr/functor/integrate_1d.hpp (4)
• M stan/math/rev/arr/functor/integrate_1d.hpp (14)
• M test/unit/math/prim/arr/functor/integrate_1d_test.cpp (2)
• M test/unit/math/rev/arr/functor/integrate_1d_test.cpp (2)
Patch Links:
• https://github.com/stan-dev/math/pull/1409.patch
• https://github.com/stan-dev/math/pull/1409.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
I'm confused - every other function in the Math library uses stream pointers right now, so it seems like the code must be handling it, right? Agreed that we should refactor to references some day, but I think it's much more important be in a consistent state. This remedies that in the shortest time. Please file an issue for refactoring the Math library to references. |
|
It looks like the Windows EC2 nodes are the culprit - @serban-nicusor-toptal I'm going to turn them off for now in Jenkins until they get sorted. |
|
This happens from time to time. Sometime it runs out of space and then can find the sundials .a file. Restarts usually solve that. |
|
|
|
Could be, either that or a mismatch of Java versions which is somethin Nic mentioned some time ago. Not sure what is the status of that. But yeah, when i see that i just restart the job or part of the job and that does the trick. It doesnt happen that often, so far it wasnt that annoying. But I dont get annoyed quickly :) |
|
Ah, now I need to update the old compiler... |
|
Is there a reason to no just switch back to stanc3 on cmdstan develop? |
|
(stat_comp_benchmarks/benchmarks/gp_pois_regr/gp_pois_regr.stan, 1.0) |
|
(stat_comp_benchmarks/benchmarks/gp_pois_regr/gp_pois_regr.stan, 1.02) |
5 participants
Summary
Minor refactor of integrate_1d to use std::ostream* instead of std::ostream& like all other functions. Likely an oversight from its initial implementation.
Checklist
Math issue: Fixes ostream argument in integrate_1d #933
Copyright holder: (fill in copyright holder information)
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses: Columbia University
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen