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

feat: expose explore eval stats to python interface #4451

Merged
merged 25 commits into from
Jun 1, 2023

Conversation

olgavrou
Copy link
Collaborator

No description provided.

ex_l_str = '{"_label_cost":-0.9,"_label_probability":0.5,"_label_Action":1,"_labelIndex":0,"o":[{"v":1.0,"EventId":"38cbf24f-70b2-4c76-aa0c-970d0c8d388e","ActionTaken":false}],"Timestamp":"2020-11-15T17:09:31.8350000Z","Version":"1","EventId":"38cbf24f-70b2-4c76-aa0c-970d0c8d388e","a":[1,2],"c":{ "GUser":{"id":"person5","major":"engineering","hobby":"hiking","favorite_character":"spock"}, "_multi": [ { "TAction":{"topic":"SkiConditions-VT"} }, { "TAction":{"topic":"HerbGarden"} } ] },"p":[0.5,0.5],"VWState":{"m":"N/A"}}\n'
ex_l = vw.parse(ex_l_str)
vw.learn(ex_l)
vw.finish_example(ex_l)
Copy link
Collaborator

Choose a reason for hiding this comment

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

you have to call finish example before fetching this metrics right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe add comment for someone checking this test to make sure that they do that

python/pylibvw.cc Outdated Show resolved Hide resolved
@rajan-chari
Copy link
Member

Wondering if there should be an explict arg asking for stats. Library can then avoid calculating stats ...

@olgavrou
Copy link
Collaborator Author

Wondering if there should be an explict arg asking for stats. Library can then avoid calculating stats ...

Explore Eval doesn't make sense without these stats really, we discussed yesterday and are going to move these to metrics so that they can be exposed to python via that

@jackgerrits
Copy link
Member

Stats cannot be skipped as GD's implementation depends on them currently.

@lalo lalo closed this Feb 14, 2023
@lalo lalo reopened this Feb 14, 2023
@@ -137,6 +137,12 @@ void predict_or_learn(metrics_data& data, T& base, E& ec)

void VW::reductions::additional_metrics(VW::workspace& all, VW::metric_sink& sink)
{
sink.set_float("sum_loss", all.sd->sum_loss);
Copy link
Collaborator

Choose a reason for hiding this comment

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

todo: shared_data should have a func to calculate this instead of being interleaved with the string output func.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jackgerrits you ran into this with py bindings next

@lalo lalo force-pushed the explore_eval_python_stats branch from 3dfdc74 to cf41995 Compare March 3, 2023 18:44
{
"id": 456,
"desc": "Spin off epsilon decay model to lower bitsize to cb_explore with explore eval",
"vw_command": "--cb_explore_adf -d train-sets/automl_spin_off.txt --noconstant -b 18 --predict_only_model --epsilon_decay --model_count 2 --explore_eval",
Copy link
Member

Choose a reason for hiding this comment

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

Is this meant to be in this PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think so

Copy link
Member

Choose a reason for hiding this comment

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

It's not related to metrics or explore eval though

@lalo lalo enabled auto-merge (squash) June 1, 2023 20:31
@lalo lalo merged commit 1cd150e into VowpalWabbit:master Jun 1, 2023
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