Keras: Saving history in a JSON file #861
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
|
Also @osanseviero do you think it makes sense to get rid of history in model card? |
There was a problem hiding this comment.
The PR Looks fine to me, thanks Merve for this PR!! @nateraw could you take a second look please?
(will later refactor the whole mixin and card creation after sprint is done)
Note from internal discussions, #847 and other things going on that refactoring the mixin and card creation is overlapping with other efforts (but also totally unrelated to this PR I think 😄 )
|
@osanseviero I find model card code not readable, and I feel like functions can be tweaked for better readability that's what the refactor I'm planning is about. But it's not prioritized anyway 🤗 |
There was a problem hiding this comment.
LGTM 🤗. In future PRs, let's try to simplify here, as this keras mixin file is getting a little hard to read haha.
Thanks for this one @merveenoyan! ❤️
| @@ -39,10 +39,13 @@ def _extract_hyperparameters_from_keras(model): | |||
| return hyperparameters | |||
|
|
|||
|
|
|||
| def _parse_model_history(model): | |||
| def _parse_model_history(model, save_directory): | |||
There was a problem hiding this comment.
I'm not in love with the fact this history writing is happening so deep in the logic here. save_pretrained_keras -> _create_model_card -> _write_metrics -> _parse_model_history is so deep in the code to just save model.history.history as a json file.
I don't want to block on this, but just wanted to point that out so we remember to put this in a nicer spot in future refactors.
There was a problem hiding this comment.
Yeah that's what I wanted to refactor as well (it disturbed me too!). I'm planning to do that after next Keras sprint.
julien-c
changed the title
Co-authored-by: Nathan Raw <nxr9266@g.rit.edu>
* added test and history saving * Update src/huggingface_hub/keras_mixin.py Co-authored-by: Nathan Raw <nxr9266@g.rit.edu> Co-authored-by: Nathan Raw <nxr9266@g.rit.edu>
This is a minor change that saves history as a JSON file, if history exists.
We check if history exists or not during parsing it to a markdown in
_parse_model_history()so I handled it there not to write repetitive checks. It can be further improved inside, just wanted to keep them separately for the sake of readability. (will later refactor the whole mixin and card creation after sprint is done) This might not be the best choice ever so feel free to criticize.Wrote a test to check if the file is there.