[RF] Follow up on RooFit multiprocessing developments#9345
Conversation
Returning a `std::shared_ptr` is unnessecarily restrictive. One should return `std::unique_ptr`, which can be implicitely moved to `std::shared_ptr`.
guitargeek
requested review from
Axel-Naumann,
bellenot,
hageboeck and
lmoneta
as code owners
This changes was motivated by an unused variable warning when building not in debug mode.
THe ZeroMQ headers are only a build-time dependency for RooFit and they should not be installed.
guitargeek
force-pushed
the
multiprocess_followup_1
branch
from
1319a98 to
c1a186d
Compare
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/default. Warnings:
|
| if (set_hwm) { | ||
| auto rc = zmq_setsockopt(*pusher, ZMQ_SNDHWM, &hwm, sizeof hwm); | ||
| assert(rc == 0); | ||
| EXPECT_EQ(zmq_setsockopt(*pusher, ZMQ_SNDHWM, &hwm, sizeof hwm), 0); |
There was a problem hiding this comment.
Ah yes, very good, I just copy pasted this from a libzmq test which didn't use GoogleTest, this makes more sense here!
egpbos
left a comment
egpbos
left a comment
There was a problem hiding this comment.
Looks good to me, and thanks for taking this up!
Would it be possible to change Jenkins configuration to build with -Droofit_multiprocess=ON -Dbuiltin_zeromq=ON -Dbuiltin_cppzmq=ON? Otherwise it has to be tested manually each time.
|
Build failed on mac1015/python3. Failing tests:
|
|
Thanks Patrick! I'll see how we can enable multiprocessing in Jenkins by default later. |
3 participants
This PR contains some follow-up changes to #8700 and #9078:
std::shared_ptrfor return values(c) CERN(requested by @Axel-Naumann in RooFit::MultiProcess & TestStatistics part 6: add RooFit::TestStatistics #8700 (comment))