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

fix: repartition bug in LightGBMRanker #1368

Merged
merged 2 commits into from
Jan 28, 2022
Merged

Conversation

liyzcj
Copy link
Contributor

@liyzcj liyzcj commented Jan 26, 2022

#1336 fix repartition bug in LightGBMRanker

@liyzcj liyzcj requested a review from imatiach-msft as a code owner January 26, 2022 16:47
@ghost
Copy link

ghost commented Jan 26, 2022

CLA assistant check
All CLA requirements met.

@imatiach-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

df.count
df
}
case Some(groupingCol) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I recall this logic was modified in this PR from another user for the ranker:
#778
who had issues with this logic, so I would just be careful here

@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2022

Codecov Report

Merging #1368 (e7c33a3) into master (be49658) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1368      +/-   ##
==========================================
- Coverage   84.77%   84.69%   -0.09%     
==========================================
  Files         287      287              
  Lines       14230    14234       +4     
  Branches      732      729       -3     
==========================================
- Hits        12064    12055       -9     
- Misses       2166     2179      +13     
Impacted Files Coverage Δ
...oft/azure/synapse/ml/lightgbm/LightGBMRanker.scala 60.56% <0.00%> (-3.62%) ⬇️
...crosoft/azure/synapse/ml/io/http/HTTPClients.scala 58.33% <0.00%> (-15.01%) ⬇️

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 be49658...e7c33a3. Read the comment docs.

@mhamilton723
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mhamilton723 mhamilton723 merged commit 802b47f into microsoft:master Jan 28, 2022
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.

4 participants