-
Notifications
You must be signed in to change notification settings - Fork 27.4k
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
Respect --ckpt-dir command-line argument when merging models #1261
Conversation
Also fixed |
you're meant to be able to input absolute filename (including extension) in there - this is what os.path.exists is checking for. If the absolute filename does not exist, then you should check cmd_opts.ckpt_dir instead of 'models/' |
and cmd_opts.ckpt_dir should be just assumed to exist |
Could you clarify? The current UI behavior seems to prevent someone from ever entering an absolute filename for a checkpoint. Can assume ckpt_dir exists though. |
oh, now i get it; it has been updated to use the dropdown and I forgot about that |
Let me test if it still works as expected with your changes- better to leave that functionality in for later, I'd imagine. EDIT: It did not work, the model output name got garbled. This should do for now. |
Alright, it works as expected now with the assumption that |
Looks like you added most of the changes to master already- don't forget the |
small dtype fix
--ckpt-dir
is now respected by the checkpoint merging code, and subdirectories are supported. Fixes #1224.