-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[R-package] [python-package] More convenient API for interaction_constraints #6376
Comments
@jameslamb could you also add a Python tag? There, we would need two things: translating feature names to int positions, and adding the set of skipped features as additional set. |
Question to @jameslamb and others: In Python, the interaction constraints are transformed into a string here, just like any other list of list-like object: https://github.com/microsoft/LightGBM/blob/master/python-package/lightgbm/basic.py, def _param_dict_to_str(data: Optional[Dict[str, Any]]) -> str:
"""Convert Python dictionary to string, which is passed to C API."""
if data is None or not data:
return ""
pairs = []
for key, val in data.items():
if isinstance(val, (list, tuple, set)) or _is_numpy_1d_array(val):
pairs.append(f"{key}={','.join(map(_to_string, val))}")
elif isinstance(val, (str, Path, _NUMERIC_TYPES)) or _is_numeric(val):
pairs.append(f"{key}={val}")
elif val is not None:
raise TypeError(f"Unknown type of parameter:{key}, got:{type(val).__name__}")
return " ".join(pairs) Is it an option to add a snippet like this? if key == "interaction_constraints":
val = _prep_interaction_constraints(vals, feature_names)
Both would need access to the list of feature_names, so we would need to either pass |
We don't actually have a |
hmmmm @btrotta @guolinke if At first I agreed with @mayer79 's assessment that they should not be excluded, but seeing this docstring makes me think they're intentionally excluded: LightGBM/include/LightGBM/config.h Line 575 in 28536a0
|
I'd prefer that that be done outside of LightGBM/python-package/lightgbm/basic.py Lines 2110 to 2130 in 28536a0
I'd support it being a private function, so it can be shared between |
I'm generally supportive of everything you've proposed at the top of this issue, thanks for working on it and for adding that context about consistency with XGBoost and scikit-learn! Let's just please wait to hear from @btrotta or @guolinke about whether omitting features not mentioned in |
Sorry, I don't remember about that 🤣, I think it was not intentional. We can align with XGBoost. |
😂 ok thank you, I don't remember either. Agree then, let's move forward @mayer79 |
Thanks for the hint. I will soon draft the function and then we determine where to place it :-). |
Interaction constraints are extremely useful in practice. The API in LightGBM has two aspects that could be improved:
[[0], [1, 2]]
. We could also allow feature names as in XGBoost, or in the R-package of LightGBM.[[0]]
, or as[["living_area"]]
.Both aspects help to reduce user mistakes and aligns the API with XGBoost (and partly with Scikit-Learn's histogram gradient boosting implementation).
I can make two PRs: one for the R-package, and one for the Python package.
@jameslamb @btrotta
The text was updated successfully, but these errors were encountered: