Skip to content

exp init: Replace live section with metrics and plots. - #7603

Merged
daavoo merged 1 commit into
mainfrom
exp-init-live
Apr 21, 2022
Merged

exp init: Replace live section with metrics and plots.#7603
daavoo merged 1 commit into
mainfrom
exp-init-live

Conversation

@daavoo

@daavoo daavoo commented Apr 20, 2022

Copy link
Copy Markdown
Contributor

Closes #7534

@daavoo
daavoo requested a review from a team as a code owner April 20, 2022 07:46
@daavoo
daavoo requested review from dberenbaum, efiop and skshetry and removed request for efiop April 20, 2022 07:46
Comment thread tests/func/experiments/test_init.py Outdated
# TODO: discuss:
# IMO, specifying `live` should not create checkpoint output
# unless type="dl" is passed
"outs": ["models"],

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.

@dberenbaum / @skshetry I might have missed the discussion about this behavior.

Enabling checkpoints makes sense with type="dl", but passing live should not mean that checkpoints are also enabled.

I have changed the previous behavior in order to decouple live from checkpoints. Checkpoints are only enabled with type="dl".

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It was introduced in #6716. I don't remember exactly why it was done this way, but probably it was done to unify with interactive mode (even with --type=dl, users can omit live).

It makes sense to do that anyway on --type="dl".

Comment thread dvc/repo/experiments/init.py
@daavoo
daavoo force-pushed the exp-init-live branch 3 times, most recently from e37298d to 51039a6 Compare April 20, 2022 09:08
@daavoo
daavoo requested a review from skshetry April 20, 2022 09:08
Comment thread dvc/repo/experiments/init.py Outdated
@daavoo
daavoo requested a review from skshetry April 20, 2022 17:11
@daavoo
daavoo merged commit 5c56157 into main Apr 21, 2022
@daavoo
daavoo deleted the exp-init-live branch April 21, 2022 06:55
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.

exp init: Revisit type="dl"

2 participants