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

objective = regression and metric = ndcg #1911

Closed
henry0312 opened this issue Dec 17, 2018 · 10 comments
Closed

objective = regression and metric = ndcg #1911

henry0312 opened this issue Dec 17, 2018 · 10 comments

Comments

@henry0312
Copy link
Contributor

henry0312 commented Dec 17, 2018

We cannot train with objective=regression and metric=ndcg.
These parameters mean that I want to minimize MSE in training and evaluate nDCG for early stopping.

Environment info

Operating System: any

CPU/GPU model:

C++/Python/R version: Python 3

Error message

❯ python regressor_ndcg.py
[LightGBM] [Fatal] label should be int type (met 15.171340) for ranking task,
for the gain of label, please set the label_gain parameter
Traceback (most recent call last):
  File "regressor_ndcg.py", line 42, in <module>
    early_stopping_rounds=10)
  File "/usr/local/var/pyenv/versions/3.6.6/Python.framework/Versions/3.6/lib/python3.6/site-packages/lightgbm/engine.py", line 195, in train
    booster = Booster(params=params, train_set=train_set)
  File "/usr/local/var/pyenv/versions/3.6.6/Python.framework/Versions/3.6/lib/python3.6/site-packages/lightgbm/basic.py", line 1512, in __init__
    ctypes.byref(self.handle)))
  File "/usr/local/var/pyenv/versions/3.6.6/Python.framework/Versions/3.6/lib/python3.6/site-packages/lightgbm/basic.py", line 46, in _safe_call
    raise LightGBMError(decode_string(_LIB.LGBM_GetLastError()))
lightgbm.basic.LightGBMError: label should be int type (met 15.171340) for ranking task,
for the gain of label, please set the label_gain parameter

Reproducible examples

regressor_ndcg.py

import numpy
import lightgbm
from lightgbm.sklearn import LGBMRegressor, LGBMModel, LGBMRanker
from sklearn.datasets import make_regression
from sklearn.model_selection import train_test_split
from sklearn.preprocessing import MinMaxScaler


X, y = make_regression(n_samples=100, n_features=10, random_state=42)
scaler = MinMaxScaler((0, 20))
y = scaler.fit_transform(y.reshape(-1, 1))
y = y.reshape(-1)

X_train, X_test, y_train, y_test = train_test_split(X, y, test_size=0.3, random_state=42)
y_test = y_test.astype(int)  # label should be int type
group_train = [10 for _ in range(7)]
group_test = [10 for _ in range(3)]

# create dataset for lightgbm
lgb_train = lightgbm.Dataset(X_train, y_train, group=group_train)
lgb_eval = lightgbm.Dataset(X_test, y_test, group=group_test)

# specify your configurations as a dict
params = {
    'boosting_type': 'gbdt',
    'objective': 'regression',
    'metric': 'ndcg',
    'eval_at': [1, 3, 5],
    'label_gain': list(range(0, numpy.max(y_test) + 1)),
    'num_leaves': 15,
    'learning_rate': 0.01,
    'feature_fraction': 0.9,
    'bagging_fraction': 0.8,
    'bagging_freq': 5,
    'seed': 42,
}

gbm = lightgbm.train(params,
                     lgb_train,
                     num_boost_round=1000,
                     valid_sets=[lgb_eval],
                     early_stopping_rounds=10)

Steps to reproduce

  1. python regressor_ndcg.py
@henry0312
Copy link
Contributor Author

I guess that in training or evaluation, LightGBM calculates nDCG for train set, but this is not needed.

@henry0312
Copy link
Contributor Author

@guolinke do you have any ideas?

@henry0312
Copy link
Contributor Author

https://github.com/Microsoft/LightGBM/blob/cba824474897c8d7e71a3df261faaefa091a40c5/src/c_api.cpp#L104-L113

Why do we need train_metric_?
Do we need just valid_metrics_ for ealy stopping?

@henry0312
Copy link
Contributor Author

@guolinke The below is a work-around.
What do you think about it?

diff --git a/python-package/lightgbm/basic.py b/python-package/lightgbm/basic.py
index b347f73..08cddcb 100644
--- a/python-package/lightgbm/basic.py
+++ b/python-package/lightgbm/basic.py
@@ -1528,7 +1528,6 @@ class Booster(object):
             # buffer for inner predict
             self.__inner_predict_buffer = [None]
             self.__is_predicted_cur_iter = [False]
-            self.__get_eval_info()
             self.pandas_categorical = train_set.pandas_categorical
         elif model_file is not None:
             # Prediction task
diff --git a/src/boosting/gbdt.cpp b/src/boosting/gbdt.cpp
index 792bd98..7ad45cb 100644
--- a/src/boosting/gbdt.cpp
+++ b/src/boosting/gbdt.cpp
@@ -537,14 +537,7 @@ std::string GBDT::OutputMetric(int iter) {
 std::vector<double> GBDT::GetEvalAt(int data_idx) const {
   CHECK(data_idx >= 0 && data_idx <= static_cast<int>(valid_score_updater_.size()));
   std::vector<double> ret;
-  if (data_idx == 0) {
-    for (auto& sub_metric : training_metrics_) {
-      auto scores = EvalOneMetric(sub_metric, train_score_updater_->score());
-      for (auto score : scores) {
-        ret.push_back(score);
-      }
-    }
-  } else {
+  if (data_idx > 0) {
     auto used_idx = data_idx - 1;
     for (size_t j = 0; j < valid_metrics_[used_idx].size(); ++j) {
       auto test_scores = EvalOneMetric(valid_metrics_[used_idx][j], valid_score_updater_[used_idx]->score());
diff --git a/src/c_api.cpp b/src/c_api.cpp
index 3a8fbe8..b567db5 100644
--- a/src/c_api.cpp
+++ b/src/c_api.cpp
@@ -100,17 +100,6 @@ public:
     if (objective_fun_ != nullptr) {
       objective_fun_->Init(train_data_->metadata(), train_data_->num_data());
     }
-
-    // create training metric
-    train_metric_.clear();
-    for (auto metric_type : config_.metric) {
-      auto metric = std::unique_ptr<Metric>(
-        Metric::CreateMetric(metric_type, config_));
-      if (metric == nullptr) { continue; }
-      metric->Init(train_data_->metadata(), train_data_->num_data());
-      train_metric_.push_back(std::move(metric));
-    }
-    train_metric_.shrink_to_fit();
   }
 
   void ResetTrainingData(const Dataset* train_data) {
@@ -299,18 +288,23 @@ public:
 
   int GetEvalCounts() const {
     int ret = 0;
-    for (const auto& metric : train_metric_) {
-      ret += static_cast<int>(metric->GetName().size());
+    for (size_t i = 0; i < valid_metrics_.size(); ++i) {
+      for (size_t j = 0; j < valid_metrics_[i].size(); ++j) {
+        ret += static_cast<int>(valid_metrics_[i][j]->GetName().size());
+      }
     }
     return ret;
   }
 
   int GetEvalNames(char** out_strs) const {
     int idx = 0;
-    for (const auto& metric : train_metric_) {
-      for (const auto& name : metric->GetName()) {
-        std::memcpy(out_strs[idx], name.c_str(), name.size() + 1);
-        ++idx;
+    for (size_t i = 0; i < valid_metrics_.size(); ++i) {
+      for (size_t j = 0; j < valid_metrics_[i].size(); ++j) {
+          auto& metric = valid_metrics_[i][j];
+          for (const auto& name : metric->GetName()) {
+            std::memcpy(out_strs[idx], name.c_str(), name.size() + 1);
+            ++idx;
+          }
       }
     }
     return idx;

@guolinke
Copy link
Collaborator

@henry0312
Current design is for dynamic support, for example, user may want to set the result over training set after 100 iterations. Therefore, we initialize them all at the beginning.

I think your fixes will broke this, since we cannot output the eval result over train set anymore.

@henry0312
Copy link
Contributor Author

Thank you for your check.

Current design is for dynamic support, for example, user may want to set the result over training set after 100 iterations. Therefore, we initialize them all at the beginning.

I see...

I think your fixes will broke this, since we cannot output the eval result over train set anymore.

How do we set metrics for train and valid set independently?

@henry0312
Copy link
Contributor Author

Here is an another patch.
This splits config_.metric into config_.train_metric and config_.valid_metric.
This will work fine only when eval_set doen't include train set, and it's hard for me to solve 😢

diff --git a/include/LightGBM/config.h b/include/LightGBM/config.h
index 7f29b05..b73a691 100644
--- a/include/LightGBM/config.h
+++ b/include/LightGBM/config.h
@@ -697,7 +697,8 @@ public:
   // descl2 = ``xentlambda``, "intensity-weighted" cross-entropy, aliases: ``cross_entropy_lambda``
   // descl2 = ``kldiv``, `Kullback-Leibler divergence <https://en.wikipedia.org/wiki/Kullback%E2%80%93Leibler_divergence>`__, aliases: ``kullback_leibler``
   // desc = support multiple metrics, separated by ``,``
-  std::vector<std::string> metric;
+  std::vector<std::string> train_metric;
+  std::vector<std::string> valid_metric;
 
   // check = >0
   // alias = output_freq
diff --git a/python-package/lightgbm/basic.py b/python-package/lightgbm/basic.py
index b347f73..f62da26 100644
--- a/python-package/lightgbm/basic.py
+++ b/python-package/lightgbm/basic.py
@@ -1708,7 +1708,7 @@ class Booster(object):
         self : Booster
             Booster with new parameters.
         """
-        if any(metric_alias in params for metric_alias in ('metric', 'metrics', 'metric_types')):
+        if any(metric_alias in params for metric_alias in ('train_metric', 'valid_metric', 'metrics', 'metric_types')):
             self.__need_reload_eval_info = True
         params_str = param_dict_to_str(params)
         if params_str:
@@ -2320,6 +2320,8 @@ class Booster(object):
                 ctypes.c_int(data_idx),
                 ctypes.byref(tmp_out_len),
                 result.ctypes.data_as(ctypes.POINTER(ctypes.c_double))))
+            print('__num_inner_eval:', self.__num_inner_eval)
+            print('tmp_out_len.value:', tmp_out_len.value)
             if tmp_out_len.value != self.__num_inner_eval:
                 raise ValueError("Wrong length of eval results")
             for i in range_(self.__num_inner_eval):
@@ -2366,7 +2368,7 @@ class Booster(object):
     def __get_eval_info(self):
         """Get inner evaluation count and names."""
         if self.__need_reload_eval_info:
-            self.__need_reload_eval_info = False
+            # self.__need_reload_eval_info = False
             out_num_eval = ctypes.c_int(0)
             # Get num of inner evals
             _safe_call(_LIB.LGBM_BoosterGetEvalCounts(
diff --git a/src/application/application.cpp b/src/application/application.cpp
index 3fbc7d3..15363ee 100644
--- a/src/application/application.cpp
+++ b/src/application/application.cpp
@@ -116,7 +116,7 @@ void Application::LoadData() {
   }
   // create training metric
   if (config_.is_provide_training_metric) {
-    for (auto metric_type : config_.metric) {
+    for (auto metric_type : config_.train_metric) {
       auto metric = std::unique_ptr<Metric>(Metric::CreateMetric(metric_type, config_));
       if (metric == nullptr) { continue; }
       metric->Init(train_data_->metadata(), train_data_->num_data());
@@ -126,7 +126,7 @@ void Application::LoadData() {
   train_metric_.shrink_to_fit();
 
 
-  if (!config_.metric.empty()) {
+  if (!config_.train_metric.empty()) {
     // only when have metrics then need to construct validation data
 
     // Add validation data, if it exists
@@ -146,7 +146,7 @@ void Application::LoadData() {
 
       // add metric for validation data
       valid_metrics_.emplace_back();
-      for (auto metric_type : config_.metric) {
+      for (auto metric_type : config_.valid_metric) {
         auto metric = std::unique_ptr<Metric>(Metric::CreateMetric(metric_type, config_));
         if (metric == nullptr) { continue; }
         metric->Init(valid_datas_.back()->metadata(),
diff --git a/src/c_api.cpp b/src/c_api.cpp
index 3a8fbe8..48a28c5 100644
--- a/src/c_api.cpp
+++ b/src/c_api.cpp
@@ -103,7 +103,7 @@ public:
 
     // create training metric
     train_metric_.clear();
-    for (auto metric_type : config_.metric) {
+    for (auto metric_type : config_.train_metric) {
       auto metric = std::unique_ptr<Metric>(
         Metric::CreateMetric(metric_type, config_));
       if (metric == nullptr) { continue; }
@@ -164,7 +164,7 @@ public:
   void AddValidData(const Dataset* valid_data) {
     std::lock_guard<std::mutex> lock(mutex_);
     valid_metrics_.emplace_back();
-    for (auto metric_type : config_.metric) {
+    for (auto metric_type : config_.valid_metric) {
       auto metric = std::unique_ptr<Metric>(Metric::CreateMetric(metric_type, config_));
       if (metric == nullptr) { continue; }
       metric->Init(valid_data->metadata(), valid_data->num_data());
@@ -302,6 +302,11 @@ public:
     for (const auto& metric : train_metric_) {
       ret += static_cast<int>(metric->GetName().size());
     }
+    for (size_t i = 0; i < valid_metrics_.size(); ++i) {
+      for (size_t j = 0; j < valid_metrics_[i].size(); ++j) {
+        ret += static_cast<int>(valid_metrics_[i][j]->GetName().size());
+      }
+    }
     return ret;
   }
 
@@ -309,10 +314,21 @@ public:
     int idx = 0;
     for (const auto& metric : train_metric_) {
       for (const auto& name : metric->GetName()) {
+        printf("%s\n", name.c_str());
         std::memcpy(out_strs[idx], name.c_str(), name.size() + 1);
         ++idx;
       }
     }
+    for (size_t i = 0; i < valid_metrics_.size(); ++i) {
+      for (size_t j = 0; j < valid_metrics_[i].size(); ++j) {
+          auto& metric = valid_metrics_[i][j];
+          for (const auto& name : metric->GetName()) {
+            printf("%s\n", name.c_str());
+            std::memcpy(out_strs[idx], name.c_str(), name.size() + 1);
+            ++idx;
+          }
+      }
+    }
     return idx;
   }
 
diff --git a/src/io/config.cpp b/src/io/config.cpp
index 414ecaf..e4d1f68 100644
--- a/src/io/config.cpp
+++ b/src/io/config.cpp
@@ -68,9 +68,9 @@ void GetObjectiveType(const std::unordered_map<std::string, std::string>& params
   }
 }
 
-void GetMetricType(const std::unordered_map<std::string, std::string>& params, std::vector<std::string>* metric) {
+void GetTrainMetricType(const std::unordered_map<std::string, std::string>& params, std::vector<std::string>* metric) {
   std::string value;
-  if (Config::GetString(params, "metric", &value)) {
+  if (Config::GetString(params, "train_metric", &value)) {
     // clear old metrics
     metric->clear();
     // to lower
@@ -91,14 +91,39 @@ void GetMetricType(const std::unordered_map<std::string, std::string>& params, s
     metric->shrink_to_fit();
   }
   // add names of objective function if not providing metric
-  if (metric->empty() && value.size() == 0) {
+  /*if (metric->empty() && value.size() == 0) {
     if (Config::GetString(params, "objective", &value)) {
       std::transform(value.begin(), value.end(), value.begin(), Common::tolower);
       metric->push_back(value);
     }
+  }*/
+}
+
+void GetValidMetricType(const std::unordered_map<std::string, std::string>& params, std::vector<std::string>* metric) {
+  std::string value;
+  if (Config::GetString(params, "valid_metric", &value)) {
+    // clear old metrics
+    metric->clear();
+    // to lower
+    std::transform(value.begin(), value.end(), value.begin(), Common::tolower);
+    // split
+    std::vector<std::string> metrics = Common::Split(value.c_str(), ',');
+    // remove duplicate
+    std::unordered_set<std::string> metric_sets;
+    for (auto& met : metrics) {
+      std::transform(met.begin(), met.end(), met.begin(), Common::tolower);
+      if (metric_sets.count(met) <= 0) {
+        metric_sets.insert(met);
+      }
+    }
+    for (auto& met : metric_sets) {
+      metric->push_back(met);
+    }
+    metric->shrink_to_fit();
   }
 }
 
+
 void GetTaskType(const std::unordered_map<std::string, std::string>& params, TaskType* task) {
   std::string value;
   if (Config::GetString(params, "task", &value)) {
@@ -164,7 +189,8 @@ void Config::Set(const std::unordered_map<std::string, std::string>& params) {
 
   GetTaskType(params, &task);
   GetBoostingType(params, &boosting);
-  GetMetricType(params, &metric);
+  GetTrainMetricType(params, &train_metric);
+  GetValidMetricType(params, &valid_metric);
   GetObjectiveType(params, &objective);
   GetDeviceType(params, &device_type);
   GetTreeLearnerType(params, &tree_learner);
@@ -215,7 +241,7 @@ void Config::CheckParamConflict() {
       Log::Fatal("Number of classes must be 1 for non-multiclass training");
     }
   }
-  for (std::string metric_type : metric) {
+  for (std::string metric_type : train_metric) {
     bool metric_custom_or_none = metric_type == std::string("none") || metric_type == std::string("null") 
                                  || metric_type == std::string("custom") || metric_type == std::string("na");
     bool metric_type_multiclass = (CheckMultiClassObjective(metric_type)
@@ -227,6 +253,19 @@ void Config::CheckParamConflict() {
       Log::Fatal("Multiclass objective and metrics don't match");
     }
   }
+  for (std::string metric_type : valid_metric) {
+    bool metric_custom_or_none = metric_type == std::string("none") || metric_type == std::string("null") 
+                                 || metric_type == std::string("custom") || metric_type == std::string("na");
+    bool metric_type_multiclass = (CheckMultiClassObjective(metric_type)
+                                   || metric_type == std::string("multi_logloss")
+                                   || metric_type == std::string("multi_error")
+                                   || (metric_custom_or_none && num_class_check > 1));
+    if ((objective_type_multiclass && !metric_type_multiclass)
+        || (!objective_type_multiclass && metric_type_multiclass)) {
+      Log::Fatal("Multiclass objective and metrics don't match");
+    }
+  }
+
 
   if (num_machines > 1) {
     is_parallel = true;
@@ -270,7 +309,8 @@ std::string Config::ToString() const {
   std::stringstream str_buf;
   str_buf << "[boosting: " << boosting << "]\n";
   str_buf << "[objective: " << objective << "]\n";
-  str_buf << "[metric: " << Common::Join(metric, ",") << "]\n";
+  str_buf << "[train_metric: " << Common::Join(train_metric, ",") << "]\n";
+  str_buf << "[valid_metric: " << Common::Join(valid_metric, ",") << "]\n";
   str_buf << "[tree_learner: " << tree_learner << "]\n";
   str_buf << "[device_type: " << device_type << "]\n";
   str_buf << SaveMembersToString();
diff --git a/src/io/config_auto.cpp b/src/io/config_auto.cpp
index 3803d98..9239eda 100644
--- a/src/io/config_auto.cpp
+++ b/src/io/config_auto.cpp
@@ -140,7 +140,8 @@ std::unordered_map<std::string, std::string> Config::alias_table({
   {"output_freq", "metric_freq"},
   {"training_metric", "is_provide_training_metric"},
   {"is_training_metric", "is_provide_training_metric"},
-  {"train_metric", "is_provide_training_metric"},
+  /* {"train_metric", "train_metric"}, */
+  /* {"valid_metric", "valid_metric"}, */
   {"ndcg_eval_at", "eval_at"},
   {"ndcg_at", "eval_at"},
   {"map_eval_at", "eval_at"},
@@ -249,7 +250,8 @@ std::unordered_set<std::string> Config::parameter_set({
   "tweedie_variance_power",
   "max_position",
   "label_gain",
-  "metric",
+  "train_metric",
+  "valid_metric",
   "metric_freq",
   "is_provide_training_metric",
   "eval_at",

@henry0312
Copy link
Contributor Author

This will work fine only when eval_set doen't include train set, and it's hard for me to solve

Finally, I solved it.

@StrikerRUS
Copy link
Collaborator

Re-opening, as we have opened PR for this feature: #1912.

@StrikerRUS
Copy link
Collaborator

Closing in favor of being in #2302. We decided to keep all feature requests in one place.

Welcome to contribute this feature! Please re-open (or post a comment if you are not a topic starter) this issue if you are actively working on implementing this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants