Skip to content

Use log_artifact in notebook.#194

Merged
daavoo merged 1 commit into
masterfrom
171-experiments-include-model-in-the-experimetns-inside-notebook
Apr 13, 2023
Merged

Use log_artifact in notebook.#194
daavoo merged 1 commit into
masterfrom
171-experiments-include-model-in-the-experimetns-inside-notebook

Conversation

@daavoo
Copy link
Copy Markdown
Contributor

@daavoo daavoo commented Apr 13, 2023

Closes #171

It is currently deployed:

Captura de pantalla 2023-04-13 a las 18 02 51

@daavoo daavoo linked an issue Apr 13, 2023 that may be closed by this pull request
@daavoo daavoo requested review from alex000kim and dberenbaum April 13, 2023 16:00
Copy link
Copy Markdown
Contributor Author

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

Actually found a bug in experiment refs in Studio (doesn't display the details of the .dvc file inside the exp ref)

cp $HERE/code/params.yaml .
sed -e "s/base_lr: 0.01/base_lr: $BEST_EXP_BASE_LR/" -i".bkp" params.yaml
rm params.yaml.bkp
dvc remove models/model.pkl.dvc
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not sure I understand the logic here, could you describe the workflow? do we transition from dvc add to log_artifact at some stage?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We transition from log_artifact which is dvc add to stage output in the dvc.yaml.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

got it ... and what it was before the log_artifact where and how did we save the model?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what it was before the log_artifact where and how did we save the model?

We didn't save it at all during the notebook state, that was the motivation for the P.R.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

got it!

@daavoo daavoo merged commit b08a8bb into master Apr 13, 2023
@daavoo daavoo deleted the 171-experiments-include-model-in-the-experimetns-inside-notebook branch April 13, 2023 17:18
Copy link
Copy Markdown

@dberenbaum dberenbaum left a comment

Choose a reason for hiding this comment

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

Thanks @daavoo! We should also add log_artifact to train.py, but it's not a blocker if we want to wait until we are ready to add type=model or other MR stuff since it should be a noop now. Do we have that tracked somewhere?

Minor non-blocking thought: the DVCLive code is starting to look a little complex. Should we add some comments to explain what's happening and make clear that you can pick and choose which calls you need?

@daavoo
Copy link
Copy Markdown
Contributor Author

daavoo commented Apr 13, 2023

We should also add log_artifact to train.py, but it's not a blocker if we want to wait until we are ready to add type=model or other MR stuff since it should be a noop now. Do we have that tracked somewhere?

Um. So the model would not be a stage output?

@dberenbaum
Copy link
Copy Markdown

No, the model can still be a stage output, that's why it would be a noop now and I think it's okay to not include it yet. Once log_artifact also adds it to the artifacts section, we will want it inside train.py also. I think it also may confuse users if log_artifact is used in the notebook but not train.py.

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.

experiments: Include model in the experimetns inside notebook

3 participants