-
Notifications
You must be signed in to change notification settings - Fork 17
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
Close #321. #424
Close #321. #424
Conversation
test/CMakeLists.txt
Outdated
@@ -134,7 +134,7 @@ LSAN_OPTIONS=suppressions=${CMAKE_CURRENT_SOURCE_DIR}/data/asan.supp) | |||
#------------------------------------------------------------------------------ | |||
add_executable(bo buffer_organizer_test.cc) | |||
target_link_libraries(bo ${LIBRT} hermes MPI::MPI_CXX | |||
$<$<BOOL:${HERMES_RPC_THALLIUM}>:thallium>) | |||
$<$<BOOL:${HERMES_RPC_THALLIUM}>:thallium> glog::glog gflags) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this test depend on gflags?
include_directories(${GLOG_INCLUDE_DIRS}) | ||
|
||
# GFLAGS | ||
find_package(gflags REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to make gflags an optional dependency, or remove it altogether. I don't use it for anything, and glog works fine without it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think glog depends on gflags.
[ 1%] Building CXX object src/CMakeFiles/hermes.dir/api/bucket.cc.o
[ 2%] Building CXX object adapter/test/stdio/CMakeFiles/stdio_adapter_test.dir/stdio_adapter_test.cpp.o
In file included from /home/runner/work/hermes/hermes/src/api/bucket.h:16,
from /home/runner/work/hermes/hermes/src/api/bucket.cc:13:
/home/runner/work/hermes/hermes/spack/opt/spack/linux-ubuntu20.04-x86_64_v4/gcc-10.3.0/glog-0.4.0-jcohf3eh7cax7d6742d6owabbwdj6464/include/glog/logging.h:86:10: fatal error: gflags/gflags.h: No such file or directory
86 | #include <gflags/gflags.h>
| ^~~~~~~~~~~~~~~~~
compilation terminated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can disable the gflags dependency via cmake. Unfortunately it looks like the glog spack package doesn't expose a variant without gflags. It hard-codes the dependency. I guess we're stuck with it for now.
src/data_placement_engine.cc
Outdated
std::string var_name {"blob_dst_" + std::to_string(i) + "_" + | ||
std::to_string(j)}; | ||
blob_fraction[i][j] = solver.MakeNumVar(0.0, 1, var_name); | ||
blob_constrt[num_constrts+i]->SetCoefficient(blob_fraction[i][j], 1); | ||
// total umber of variables = number of blobs * number of targets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'umber' -> 'number'
Replace OR-TOOLS with GLPK.