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

[docs] fix docs for machine_list_filename param #3863

Merged
merged 2 commits into from
Jan 28, 2021
Merged

Conversation

StrikerRUS
Copy link
Collaborator

I might be wrong, but seems machine_list_filename parameter cannot be used via C API and in consequence by any language wrapper.
@guolinke Could you please review this PR?

_safe_call(_LIB.LGBM_NetworkInit(c_str(machines),
ctypes.c_int(local_listen_port),
ctypes.c_int(listen_time_out),
ctypes.c_int(num_machines)))

/*!
* \brief Initialize the network.
* \param machines List of machines in format 'ip1:port1,ip2:port2'
* \param local_listen_port TCP listen port for local machines
* \param listen_time_out Socket time-out in minutes
* \param num_machines Total number of machines
* \return 0 when succeed, -1 when failure happens
*/
LIGHTGBM_C_EXPORT int LGBM_NetworkInit(const char* machines,
int local_listen_port,
int listen_time_out,
int num_machines);

LightGBM/src/c_api.cpp

Lines 2302 to 2316 in 4ae4abb

int LGBM_NetworkInit(const char* machines,
int local_listen_port,
int listen_time_out,
int num_machines) {
API_BEGIN();
Config config;
config.machines = RemoveQuotationSymbol(std::string(machines));
config.local_listen_port = local_listen_port;
config.num_machines = num_machines;
config.time_out = listen_time_out;
if (num_machines > 1) {
Network::Init(config);
}
API_END();
}

void Network::Init(Config config) {
if (config.num_machines > 1) {
linkers_.reset(new Linkers(config));

Linkers::Linkers(Config config) {
is_init_ = false;
// start up socket
TcpSocket::Startup();
network_time_ = std::chrono::duration<double, std::milli>(0);
num_machines_ = config.num_machines;
local_listen_port_ = config.local_listen_port;
socket_timeout_ = config.time_out;
rank_ = -1;
// parse clients from file
ParseMachineList(config.machines, config.machine_list_filename);

void Linkers::ParseMachineList(const std::string& machines, const std::string& filename) {
std::vector<std::string> lines;
if (machines.empty()) {
TextReader<size_t> machine_list_reader(filename.c_str(), false);
machine_list_reader.ReadAllLines();
if (machine_list_reader.Lines().empty()) {
Log::Fatal("Machine list file %s doesn't exist", filename.c_str());
}
lines = machine_list_reader.Lines();
} else {
lines = Common::Split(machines.c_str(), ',');
}

Copy link
Collaborator

@guolinke guolinke left a comment

Choose a reason for hiding this comment

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

yeah, the filename is for CLI only.

@jameslamb jameslamb merged commit 8ef874b into master Jan 28, 2021
@jameslamb
Copy link
Collaborator

Thanks for your attention to detail @StrikerRUS !

@jameslamb jameslamb deleted the mlist_file branch January 28, 2021 04:20
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants