Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[World Logs] Don't crash if a value can't be serialized #3591

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

spencerp
Copy link
Contributor

Patch description
text_vec and full_text_vec in the observations contain tensor and can't be json serialized in eval_model --world-logs. In this type of scenario, if we crash, we lose all of the logs and have to rerun. This fails more gracefully.

Testing steps

>>> d = {'a': tensor}
>>> json.dumps(d, default=lambda v: '<not serializable>')
'{"a": "<not serializable>"}'

Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

hmm i guess this is good, vaguely wish we could filter instead of dumping a thousand "not serializable" strings

@spencerp
Copy link
Contributor Author

hmm i guess this is good, vaguely wish we could filter instead of dumping a thousand "not serializable" strings

We could filter them. My thought was that it'd be useful to have this feedback in case you did want a field but forgot to convert it into a serializable form. Like if you put in a metric but forgot to call .value.

@stephenroller
Copy link
Contributor

We can go with this and see if gets in the way

@spencerp spencerp merged commit 2f48dcd into master Apr 15, 2021
@spencerp spencerp deleted the world-stats-json-crsh branch April 15, 2021 13:46
@spencerp spencerp mentioned this pull request Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants