Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feature: Add true streaming APIs to reduce client-side memory usage #5299
feature: Add true streaming APIs to reduce client-side memory usage #5299
Changes from 28 commits
c2c3632
6456bb4
f3db2f5
5224234
0d2c2f7
cd566c5
c74a91e
da83bfb
9f17b39
1164bb8
e0e9a68
06e5021
3b254d0
1cc4d7c
6769682
eb1deab
b0ee60c
a08a493
d533a06
c50f419
7b1bd50
7458ac4
780cd7f
4107ffb
f60842a
7a29a3b
16ee03e
958ea22
386978c
be3c368
cef90e9
91ca0b7
7adbde3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
num_init_score_
always set to non zero values?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, sometimes there are no initial scores. Should we return 1 even in the case of no initial score inputs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but when there is no num_init_score_, num_classes could be > 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, makes sense to return 1 for all non-multiclass scenarios, so I made the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that when multiclass, and
num_init_score_ == 0
, this could still return 1?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svotaw is this addressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like I missed this one. Is there a particular concern/bug? What is it that you suggest it should return? This is currently used as part of allocation. If num_init_score_ == 0, this is not used. Happy to fix it if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num_init_score_
does not always have values.I believe the
num_class
is a hyper-parameter, which can get from config. So I am not sure why you usenum_init_score_
to getnum_class
.If
dataset->num_classes()
is currently only used for init_score assignment, maybe we should use a different name to avoid future bugs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense. How about num_init_score_classes()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good to me