FairGBM implementation #1
Conversation
There was a problem hiding this comment.
Nothing major to change, overall looks good. There are two types of comments. Some explicitly say that they are to be tackled later because of our urgency.
In those cases you should open an issue per each of those comments so we don't lose them.
There is major refactor and optimization work ahead in include/LightGBM/objective_function.h, so I'm counting on keeping it as it is for now, as it seems to work, and we'll tackle this in stage 2.
There was a problem hiding this comment.
Just finished reviewing all the feedback. It was very thorough, thanks!
I answered all comments before resolving them, so please take a look at the answers and feel free to unresolve any threads.
I also left a few unresolved comments, which are yet unaddressed.
TODO: one major thing that I have to do is to move all config changes to the config.cpp and generate the correct config_auto.cpp - Issue #3
|
All "<10 lines" code changes applied. I left major refactors to a couple of weeks from now, but all should have some corresponding issue on GH. |
|
All comments reviewed; ping me before you merge! |
AndreFCruz
force-pushed
the
bulk-fairgbm-modifications
branch
2 times, most recently
from
f74ff6f
to
52211ce
Compare
AndreFCruz
force-pushed
the
bulk-fairgbm-modifications
branch
from
7db0a5e
to
61621b2
Compare
Applied all code changes pertaining to the FairGBM implementation.
Let me know if you have any questions!