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

Pin LightGBM version to remove bug from docs #1711

Merged
merged 6 commits into from Jan 21, 2021
Merged

Conversation

bchen1116
Copy link
Contributor

@bchen1116 bchen1116 commented Jan 20, 2021

fix #1659

Versions 3.1.0 and above raise the bug we see in the docs, which isn't reproducible elsewhere. The bug fix, which they filed here and addressed here, hasn't been released yet.

Using the current branch to build the docs in circleci (through ssh), our doc build passes without any errors. This suggests that once LightGBM releases a new version, we can unpin the LGBM version and remove the bug.
image

In comparison, this is what the bug looks like (using LightGBM version 3.1.1)
image

Updated docs here

EDIT
Here's a stacktrace of the docs during the build:
image
Looks like the booster update for LGBM fails, causing it to raise this error.

We don't get the same error when running the code locally in Jupyter. We do get the error when building the docs locally, which means it could be an error caused through Sphinx? But I think the easiest solution is to wait for the newest release.

@bchen1116 bchen1116 self-assigned this Jan 20, 2021
@codecov
Copy link

codecov bot commented Jan 20, 2021

Codecov Report

Merging #1711 (4b9237e) into main (4ff4aae) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1711   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         242      242           
  Lines       19075    19075           
=======================================
  Hits        19067    19067           
  Misses          8        8           

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 4ff4aae...4b9237e. Read the comment docs.

@bchen1116 bchen1116 changed the title Cap LightGBM version for docs Pin LightGBM version to remove bug from docs Jan 20, 2021
Copy link
Contributor

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@bchen1116 Looks good! I think the only thing is that we need to remember to uncap this once the latest version is published 😆

@@ -37,6 +37,7 @@ Release Notes
* Added labeling to ``graph_confusion_matrix`` :pr:`1632`
* Rerunning search for ``AutoMLSearch`` results in a message thrown rather than failing the search, and removed ``has_searched`` property :pr:`1647`
* Changed tuner class to allow and ignore single parameter values as input :pr:`1686`
* Capped LightGBM version limit to remove bug in docs :pr:`1711`
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why this is only reproducible in the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure why, but when I ran locally through Jupyter, I didn't run into this bug, even when switching versions for LightGBM. I can look a little more tomorrow and see if there's a reason though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe looking at the entire stack trace will be helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the more complete stacktrace in the description box. I suspect something in nbsphinx might be causing the bug to appear, but I'm unable to repro it in Jupyter.

Copy link
Contributor

@angela97lin angela97lin left a comment

Choose a reason for hiding this comment

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

Thank you for doing this! 👍 If there isn't already an issue filed for this, can you file an issue to add support for >=3.1.0 so we can keep track? :D

@bchen1116
Copy link
Contributor Author

Filed new issue to track unpinning the version here

@bchen1116 bchen1116 marked this pull request as ready for review January 21, 2021 15:07
@bchen1116 bchen1116 merged commit 19b1b6e into main Jan 21, 2021
@bchen1116 bchen1116 mentioned this pull request Jan 26, 2021
@freddyaboulton freddyaboulton deleted the bc_1659_lgbm_ver branch May 13, 2022 15:00
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.

LightGBM error in cost_benefit_matrix demo
3 participants