Skip to content

LightGBM model locale fix#53

Merged
AlbertoEAF merged 32 commits intomasterfrom
ft-af-lgbm-model-locale-fix
Sep 30, 2020
Merged

LightGBM model locale fix#53
AlbertoEAF merged 32 commits intomasterfrom
ft-af-lgbm-model-locale-fix

Conversation

@AlbertoEAF
Copy link
Copy Markdown
Contributor

@AlbertoEAF AlbertoEAF commented Sep 16, 2020

This implements the fix for model locale with our own patch to LightGBM v3.0.0.
Such patch currently lives at feedzai/LightGBM as it wasn't yet merged to Microsoft's mainline code.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 25, 2020

Codecov Report

Merging #53 into master will increase coverage by 0.78%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #53      +/-   ##
============================================
+ Coverage     78.46%   79.25%   +0.78%     
- Complexity      369      382      +13     
============================================
  Files            39       39              
  Lines          1333     1364      +31     
  Branches        123      127       +4     
============================================
+ Hits           1046     1081      +35     
+ Misses          221      215       -6     
- Partials         66       68       +2     
Impacted Files Coverage Δ Complexity Δ
...eedzai/openml/provider/lightgbm/SWIGResources.java 83.54% <0.00%> (-1.75%) 19.00% <0.00%> (+2.00%) ⬇️
...er/lightgbm/LightGBMBinaryClassificationModel.java 59.09% <0.00%> (+1.94%) 9.00% <0.00%> (+1.00%)
...openml/provider/lightgbm/LightGBMModelCreator.java 87.09% <0.00%> (+3.09%) 30.00% <0.00%> (+8.00%)
...feedzai/openml/provider/lightgbm/LightGBMSWIG.java 81.25% <0.00%> (+15.29%) 12.00% <0.00%> (+2.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c752c68...c603135. Read the comment docs.

Comment thread openml-lightgbm/lightgbm-provider/pom.xml Outdated
Comment thread openml-lightgbm/lightgbm-builder/pom.xml Outdated
@AlbertoEAF AlbertoEAF marked this pull request as ready for review September 25, 2020 16:22
@AlbertoEAF
Copy link
Copy Markdown
Contributor Author

AlbertoEAF commented Sep 25, 2020

I actually have one question @shenggwang @paulojrp , I added the folder standard_code_models with two model files.
Should we:

  1. Use git lfs to store them?
  2. Move this folder to the test resources as well? Right now it sits randomly

Finally, the tests generate a new folder: new_code_models, should we erase it or move it somewhere else somehow? It's ugly where it sits after you run tests. Put it in tmp? If the test fails we should not erase that folder or we can't diagnose what's wrong right?

Comment thread openml-lightgbm/lightgbm-builder/make.sh
Comment thread openml-lightgbm/lightgbm-provider/pom.xml Outdated
Comment thread openml-lightgbm/lightgbm-provider/diff_models.py
Comment thread openml-lightgbm/lightgbm-provider/standard_code_models/extend_model.py Outdated
Comment thread openml-lightgbm/lightgbm-builder/pom.xml Outdated
Comment thread openml-lightgbm/lightgbm-builder/pom.xml Outdated
@AlbertoEAF
Copy link
Copy Markdown
Contributor Author

AlbertoEAF commented Sep 29, 2020

@shenggwang do we need the dummy lightgbm-builder/javadoc/README.md in the lightgbm-builder or can I delete it?

Comment thread openml-lightgbm/lightgbm-provider/pom.xml
@shengwangsw
Copy link
Copy Markdown
Contributor

shengwangsw commented Sep 29, 2020

@shenggwang do we need the dummy lightgbm-builder/javadoc/README.md in the lightgbm-builder or can I delete it?

There is no javadoc/README.md
I think you are outdated 😄

See https://github.com/feedzai/feedzai-openml-java/tree/master/openml-lightgbm/lightgbm-builder

@AlbertoEAF AlbertoEAF merged commit 83ae7a2 into master Sep 30, 2020
@AlbertoEAF AlbertoEAF deleted the ft-af-lgbm-model-locale-fix branch September 30, 2020 11:49
jlazevedo pushed a commit that referenced this pull request Dec 2, 2020
This implements the fix for model locale with Feedzai's patch to LightGBM v3.0.0.
Such patch currently lives at feedzai/LightGBM as it wasn't yet merged to Microsoft's mainline code.

Also, it integrates the latest version of make-lightgbm with support for build caches to speed up repeated builds of LightGBM.
jlazevedo pushed a commit that referenced this pull request Dec 2, 2020
This implements the fix for model locale with Feedzai's patch to LightGBM v3.0.0.
Such patch currently lives at feedzai/LightGBM as it wasn't yet merged to Microsoft's mainline code.

Also, it integrates the latest version of make-lightgbm with support for build caches to speed up repeated builds of LightGBM.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants