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

run/repro: add option to not remove outputs before reproduction #1214

Closed
efiop opened this issue Oct 12, 2018 · 18 comments · Fixed by #1759
Closed

run/repro: add option to not remove outputs before reproduction #1214

efiop opened this issue Oct 12, 2018 · 18 comments · Fixed by #1759
Assignees
Labels
enhancement Enhances DVC p1-important Important, aka current backlog of things to do
Milestone

Comments

@efiop
Copy link
Contributor

efiop commented Oct 12, 2018

https://discordapp.com/channels/485586884165107732/485596304961962003/540232443769258014
https://discordapp.com/channels/485586884165107732/485586884165107734/547430257821483008
https://discordapp.com/channels/485586884165107732/485596304961962003/557823591379369994

@efiop efiop added the enhancement Enhances DVC label Oct 12, 2018
@efiop efiop added this to the Queue milestone Oct 12, 2018
@efiop efiop self-assigned this Oct 12, 2018
@efiop efiop removed their assignment Nov 1, 2018
@ghost
Copy link

ghost commented Nov 20, 2018

What about corrupting the cache, @efiop ? Is this for reflink only?

@efiop
Copy link
Contributor Author

efiop commented Nov 20, 2018

@MrOutis Great point! We should dvc unprotect the outputs when this option is specified, so that user has safe copies(or reflinks).

@AlJohri
Copy link

AlJohri commented Jan 30, 2019

Adding my +1 here. This would be helpful for doing a warm start during parameter search. Some of our stages are very long running and starting from the previous results would help it complete much faster.

@guysmoilov
Copy link
Contributor

@AlJohri WDYT of this (hacky) possible solution?
Add a stage before the training stage, that takes the latest cached checkpoint from the output of the training stage, moves or copies it to a different path, and then that resume checkpoint is a cached dependency of the training stage?

This should allow each git commit to describe an accurate snapshot of a training run - the checkpoint you start from, and the checkpoint that resulted.
image

@efiop efiop added the p1-important Important, aka current backlog of things to do label Mar 20, 2019
@efiop
Copy link
Contributor Author

efiop commented Mar 20, 2019

Probably should look something like:

$ dvc run --outs-no-remove ckpt ...

which would add something like

remove: False

to the dvc file for ckpt output, so that dvc repro knows about it.

@pared
Copy link
Contributor

pared commented Mar 25, 2019

Should be closed by #1759

@pared pared closed this as completed Mar 25, 2019
@pared
Copy link
Contributor

pared commented Mar 25, 2019

@AlJohri there is new option in run for this case, Ill add it to docs soon, but you can find it in closing issue name.

@piojanu
Copy link

piojanu commented Mar 26, 2019

What should I do if from time to time I want to start fresh? Use dvc remove on .dvc file or rm the output?
EDIT: dvc remove doesn't remove persistent outputs (though some progress bars appear the first time it is called, but if executed again the command does nothing). Is it expected behaviour?

@pared
Copy link
Contributor

pared commented Mar 26, 2019

@piojanu actually what this option is doing is just setting persist flag inside stage file. So if you want to edit behaviour after some time, just change this flag in .dvc file. As to dvc remove, I am looking into that.

@pared
Copy link
Contributor

pared commented Mar 26, 2019

@piojanu thank you for pointing that out! I created issue for that one. #1784

@pared
Copy link
Contributor

pared commented Mar 26, 2019

@piojanu It is also worth noting, that this behaviour is only possible, if user actually appends data to output. If, for example, our run command somehow destroys file, persist flag will not be able to prevent that. For example, reproducing given command:
dvc run --outs-persist something "echo something > something"
will always result in overwriting the file, beause that is how > works. However, if we use ">>",
then we can utilize persist functionality.

@piojanu
Copy link

piojanu commented Mar 26, 2019

Yea, I understand that :) But maybe it is worth including it in docs. Also, should dvc remove (when fixed) work or it is not recommended path and it can break something?

@pared
Copy link
Contributor

pared commented Mar 26, 2019

@piojanu it should work, even now, since patch entered master, it was my mistake, didn't test whether remove works for persistent outputs.

@guysmoilov
Copy link
Contributor

Very good guys! This is a very important feature, I already started using it to great effect instead of my above workaround: https://dagshub.com/Guy/fairseq/src/dvc/dvc-example/train.dvc

I vote for having a short flag for it, I think it will be commonly used for anyone that wants to train with checkpoints, or for scenarios like this: https://discuss.dvc.org/t/version-checking-with-dvc/168/5

Maybe -p -P ?

@efiop
Copy link
Contributor Author

efiop commented Apr 2, 2019

@guysmoilov Thanks a lot for your input on discuss forum! 🙂

Mind creating a feature request for it? 🙂

@shcheklein
Copy link
Member

@AlJohri quick question, but the "warm start" and "reusing previous results" - are you referring to reusing the previous model (trained with a different set of params) to continue training it with a new set? Or is it something different in your case? We would really appreciate your input here! Thanks!

@AlJohri
Copy link

AlJohri commented Apr 11, 2019

hey there, sorry I'm actually not 100% sure what I meant before. it was related to more efficient hyperparameter searching but I can't remember the particular use case. I had a job where the parameter search took a very long time but I don't recall how I thought a warm start might solve that in this scenario

"warm start machine learning" has a lot of hits on google so perhaps you can read more about the more general use case

@ghost
Copy link

ghost commented Apr 22, 2019

Hello, @guysmoilov , @AlJohri, @pared , @efiop !
I was thinking about the usefulness of having a persist flag and the reason behind it, after reading the whole convo the use case is still not clear for me.

The idea to resume a process from a certain point could be handled by the script itself (writing to a temporary file not specified on the run command as output and then, when the process has finished, move the temporary file to the wanted location).

It would be great if you can come up with an example of how you are using this feature today 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p1-important Important, aka current backlog of things to do
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants