-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Move deletion logic to destructor of vw #2399
Move deletion logic to destructor of vw #2399
Conversation
vowpalwabbit/global_data.cc
Outdated
} | ||
for (size_t i = 0; i < final_prediction_sink.size(); i++) | ||
if (final_prediction_sink[i] != 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.
nit: What's our bracket style, feel like this is unnecessary might as well change it up in this pr? also for each loop?
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.
Kind of depends who you ask. I prefer brackets always though personally. Good point, can definitely be a for each.
IGNORE_DEPRECATED_USAGE_END | ||
|
||
vw::~vw() |
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.
did you give this a pass to check if we are missing something that got added to the class but not to the old parse args destructor logic? feel that causes room for mismatch
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.
Yeah I did a pass and deprecated an unused variable. searchstr
seems fishy to me but don't want to deal with it in this PR
No description provided.