Skip to content

Commit

Permalink
feat: Networking compile time flag (VowpalWabbit#4571)
Browse files Browse the repository at this point in the history
  • Loading branch information
olgavrou authored Apr 24, 2023
1 parent 19fc708 commit eb15bf9
Show file tree
Hide file tree
Showing 11 changed files with 110 additions and 26 deletions.
3 changes: 2 additions & 1 deletion cmake/VowpalWabbitFeatures.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
# - The cmake variable VW_FEAT_X is set to ON, otherwise it is OFF
# - The C++ macro VW_FEAT_X_ENABLED is defined if the feature is enabled, otherwise it is not defined

set(VW_ALL_FEATURES "CSV;FLATBUFFERS;LDA;CB_GRAPH_FEEDBACK;SEARCH;LAS_SIMD")
set(VW_ALL_FEATURES "CSV;FLATBUFFERS;LDA;CB_GRAPH_FEEDBACK;SEARCH;LAS_SIMD;NETWORKING")

option(VW_FEAT_FLATBUFFERS "Enable flatbuffers support" OFF)
option(VW_FEAT_CSV "Enable csv parser" OFF)
option(VW_FEAT_CB_GRAPH_FEEDBACK "Enable cb with graph feedback reduction" OFF)
option(VW_FEAT_LDA "Enable lda reduction" ON)
option(VW_FEAT_SEARCH "Enable search reductions" ON)
option(VW_FEAT_LAS_SIMD "Enable large action space with explicit simd (only works with linux for now)" ON)
option(VW_FEAT_NETWORKING "Enable daemon mode, spanning tree, sender, and active" ON)

# Legacy options for feature enablement
if(DEFINED BUILD_FLATBUFFERS)
Expand Down
28 changes: 28 additions & 0 deletions test/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,7 @@ def convert_to_test_data(
vw_bin: str,
spanning_tree_bin: Optional[Path],
skipped_ids: List[int],
skip_network_tests: bool,
extra_vw_options: str,
) -> List[TestData]:
results: List[TestData] = []
Expand Down Expand Up @@ -895,6 +896,14 @@ def convert_to_test_data(
skip = True
skip_reason = "Test using spanning_tree skipped because of --skip_spanning_tree_tests argument"

if skip_network_tests and (
"daemon" in test["bash_command"]
or "spanning_tree" in test["bash_command"]
or "sender_test.py" in test["bash_command"]
or "active_test.py" in test["bash_command"]
):
skip = True
skip_reason = "Tests requiring daemon or network skipped because of --skip_network_tests argument"
elif "vw_command" in test:
command_line = f"{vw_bin} {test['vw_command']} {extra_vw_options}"
else:
Expand Down Expand Up @@ -1029,6 +1038,11 @@ def main():
help="Skip tests that use spanning tree",
action="store_true",
)
parser.add_argument(
"--skip_network_tests",
help="Skip all tests that require daemon or network connection",
action="store_true",
)
parser.add_argument(
"--skip_test",
help="Skip specific test ids",
Expand Down Expand Up @@ -1077,6 +1091,9 @@ def main():
)
args = parser.parse_args()

if args.skip_network_tests:
args.skip_spanning_tree_tests = True

# user did not supply dir
temp_working_dir: Path = Path(args.working_dir)
if args.for_flatbuffers and args.working_dir == str(working_dir):
Expand Down Expand Up @@ -1149,6 +1166,7 @@ def main():
vw_bin,
spanning_tree_bin,
args.skip_test,
args.skip_network_tests,
extra_vw_options=args.extra_options,
)

Expand Down Expand Up @@ -1198,6 +1216,16 @@ def main():
tests, to_flatbuff_bin, test_base_working_dir, color_enum
)

if args.skip_network_tests:
for test in tests:
if (
"--active" in test.command_line
or "--sendto" in test.command_line
or "--daemon" in test.command_line
):
test.skip = True
test.skip_reason = "Tests requiring daemon or network skipped because of --skip_network_tests argument"

tasks: List[Future[TestOutcome]] = []
completed_tests = Completion()

Expand Down
9 changes: 9 additions & 0 deletions test/run_tests_model_gen_and_load.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ def get_tests(
explicit_tests: Optional[List[int]] = None,
color_enum: Type[Union[Color, NoColor]] = Color,
skip_missing_args: bool = False,
skip_network_tests: bool = False,
skip_pr_tests: List[int] = [],
) -> List[TestData]:
test_ref_dir: Path = Path(__file__).resolve().parent
Expand All @@ -209,6 +210,7 @@ def get_tests(
vw_bin="",
spanning_tree_bin=None,
skipped_ids=[],
skip_network_tests=skip_network_tests,
extra_vw_options="",
)
filtered_tests = []
Expand Down Expand Up @@ -365,6 +367,12 @@ def main():
help="Skip specific tests based on Pull Request description",
)

parser.add_argument(
"--skip_network_tests",
help="Skip all tests that require daemon or network connection",
action="store_true",
)

args = parser.parse_args()
if args.skip_pr_tests and "skip:" in args.skip_pr_tests[0]:
skip_pr_tests = [
Expand Down Expand Up @@ -395,6 +403,7 @@ def main():
args.test,
color_enum,
args.skip_missing_args,
args.skip_network_tests,
skip_pr_tests,
)

Expand Down
6 changes: 4 additions & 2 deletions vowpalwabbit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ add_subdirectory(io)
add_subdirectory(json_parser)
add_subdirectory(model_merger)
add_subdirectory(slim)
add_subdirectory(spanning_tree_bin)
add_subdirectory(spanning_tree)
if(VW_FEAT_NETWORKING)
add_subdirectory(spanning_tree_bin)
add_subdirectory(spanning_tree)
endif()
add_subdirectory(text_parser)
if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME AND BUILD_TESTING)
add_subdirectory(test_common)
Expand Down
24 changes: 16 additions & 8 deletions vowpalwabbit/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ set(vw_core_headers
include/vw/core/cb_graph_feedback_reduction_features.h
include/vw/core/multi_ex.h
include/vw/core/learner_fwd.h
include/vw/core/daemon_utils.h
include/vw/core/cb_type.h
include/vw/core/cb.h
include/vw/core/cb_with_observations_label.h
Expand Down Expand Up @@ -65,7 +64,6 @@ set(vw_core_headers
include/vw/core/multiclass.h
include/vw/core/multilabel.h
include/vw/core/named_labels.h
include/vw/core/network.h
include/vw/core/no_label.h
include/vw/core/numeric_casts.h
include/vw/core/object_pool.h
Expand All @@ -85,7 +83,6 @@ set(vw_core_headers
include/vw/core/reduction_features.h
include/vw/core/reduction_stack.h
include/vw/core/reductions/active_cover.h
include/vw/core/reductions/active.h
include/vw/core/reductions/audit_regressor.h
include/vw/core/reductions/autolink.h
include/vw/core/reductions/automl.h
Expand Down Expand Up @@ -160,7 +157,6 @@ set(vw_core_headers
include/vw/core/reductions/recall_tree.h
include/vw/core/reductions/sample_pdf.h
include/vw/core/reductions/scorer.h
include/vw/core/reductions/sender.h
include/vw/core/reductions/shared_feature_merger.h
include/vw/core/reductions/slates.h
include/vw/core/reductions/stagewise_poly.h
Expand Down Expand Up @@ -227,7 +223,6 @@ set(vw_core_sources
src/multiclass.cc
src/multilabel.cc
src/named_labels.cc
src/network.cc
src/no_label.cc
src/parse_args.cc
src/parse_example_json.cc
Expand All @@ -241,7 +236,6 @@ set(vw_core_sources
src/qr_decomposition.cc
src/reduction_stack.cc
src/reductions/active_cover.cc
src/reductions/active.cc
src/reductions/audit_regressor.cc
src/reductions/autolink.cc
src/reductions/automl.cc
Expand Down Expand Up @@ -293,7 +287,6 @@ set(vw_core_sources
src/reductions/details/automl/automl_util.cc
src/reductions/ect.cc
src/reductions/eigen_memory_tree.cc
src/daemon_utils.cc
src/reductions/epsilon_decay.cc
src/reductions/explore_eval.cc
src/reductions/freegrad.cc
Expand Down Expand Up @@ -325,7 +318,6 @@ set(vw_core_sources
src/reductions/recall_tree.cc
src/reductions/sample_pdf.cc
src/reductions/scorer.cc
src/reductions/sender.cc
src/reductions/shared_feature_merger.cc
src/reductions/slates.cc
src/reductions/stagewise_poly.cc
Expand Down Expand Up @@ -376,6 +368,22 @@ if(VW_FEAT_CB_GRAPH_FEEDBACK)
list(APPEND vw_core_sources src/reductions/cb/cb_explore_adf_graph_feedback.cc)
endif()

if(VW_FEAT_NETWORKING)
list(APPEND vw_core_headers
include/vw/core/daemon_utils.h
include/vw/core/reductions/sender.h
include/vw/core/network.h
include/vw/core/reductions/active.h
)

list(APPEND vw_core_sources
src/daemon_utils.cc
src/reductions/sender.cc
src/network.cc
src/reductions/active.cc
)
endif()

vw_add_library(
NAME "core"
TYPE "STATIC_ONLY"
Expand Down
2 changes: 2 additions & 0 deletions vowpalwabbit/core/include/vw/core/global_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ class reduction_state
class runtime_config
{
public:
#ifdef VW_FEAT_NETWORKING_ENABLED
bool daemon;
#endif
bool vw_is_main = false; // true if vw is executable; false in library mode
bool training; // Should I train if lable data is available?
size_t pass_length;
Expand Down
2 changes: 2 additions & 0 deletions vowpalwabbit/core/include/vw/core/parse_args.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace details
class input_options
{
public:
#ifdef VW_FEAT_NETWORKING_ENABLED
bool daemon;
bool foreground;
uint32_t port;
Expand All @@ -27,6 +28,7 @@ class input_options
// If a model was saved in daemon or active learning mode, force it to accept
// local input when loaded instead.
bool no_daemon = false;
#endif

bool cache;
std::vector<std::string> cache_files;
Expand Down
2 changes: 2 additions & 0 deletions vowpalwabbit/core/src/global_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,9 @@ workspace::workspace(VW::io::logger logger) : options(nullptr, nullptr), logger(
reduction_state.active = false;
initial_weights_config.num_bits = 18;
runtime_config.default_bits = true;
#ifdef VW_FEAT_NETWORKING_ENABLED
runtime_config.daemon = false;
#endif
output_model_config.save_resume = true;
output_model_config.preserve_performance_counters = false;

Expand Down
9 changes: 8 additions & 1 deletion vowpalwabbit/core/src/parse_args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,9 @@ VW::details::input_options parse_source(VW::workspace& all, options_i& options)
VW::details::input_options parsed_options;

option_group_definition input_options("Input");
input_options.add(make_option("data", all.parser_runtime.data_filename).short_name("d").help("Example set"))
input_options
.add(make_option("data", all.parser_runtime.data_filename).short_name("d").help("Example set"))
#ifdef VW_FEAT_NETWORKING_ENABLED
.add(make_option("daemon", parsed_options.daemon).help("Persistent daemon mode on port 26542"))
.add(make_option("foreground", parsed_options.foreground)
.help("In persistent daemon mode, do not run in the background"))
Expand All @@ -387,6 +389,7 @@ VW::details::input_options parse_source(VW::workspace& all, options_i& options)
.help("Number of children for persistent daemon mode"))
.add(make_option("pid_file", parsed_options.pid_file).help("Write pid file in persistent daemon mode"))
.add(make_option("port_file", parsed_options.port_file).help("Write port used in persistent daemon mode"))
#endif
.add(make_option("cache", parsed_options.cache).short_name("c").help("Use a cache. The default is <data>.cache"))
.add(make_option("cache_file", parsed_options.cache_files).help("The location(s) of cache_file"))
.add(make_option("json", parsed_options.json).help("Enable JSON parsing"))
Expand All @@ -400,9 +403,11 @@ VW::details::input_options parse_source(VW::workspace& all, options_i& options)
"use gzip format whenever possible. If a cache file is being created, this option creates a "
"compressed cache file. A mixture of raw-text & compressed inputs are supported with autodetection."))
.add(make_option("no_stdin", parsed_options.stdin_off).help("Do not default to reading from stdin"))
#ifdef VW_FEAT_NETWORKING_ENABLED
.add(make_option("no_daemon", parsed_options.no_daemon)
.help("Force a loaded daemon or active learning model to accept local input instead of starting in "
"daemon mode"))
#endif
.add(make_option("chain_hash", parsed_options.chain_hash_json)
.keep()
.help("Enable chain hash in JSON for feature name and string feature value. e.g. {'A': {'B': 'C'}} is "
Expand Down Expand Up @@ -435,13 +440,15 @@ VW::details::input_options parse_source(VW::workspace& all, options_i& options)
}
}

#ifdef VW_FEAT_NETWORKING_ENABLED
if (parsed_options.daemon || options.was_supplied("pid_file") ||
(options.was_supplied("port") && !all.reduction_state.active))
{
all.runtime_config.daemon = true;
// allow each child to process up to 1e5 connections
all.runtime_config.numpasses = static_cast<size_t>(1e5);
}
#endif

// Add an implicit cache file based on the data filename.
if (parsed_options.cache) { parsed_options.cache_files.push_back(all.parser_runtime.data_filename + ".cache"); }
Expand Down
Loading

0 comments on commit eb15bf9

Please sign in to comment.