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

Model.load_from_checkpoint tries to open file path as URL and fail #2243

Closed
Molaire opened this issue Jun 18, 2020 · 7 comments · Fixed by #2244 or #2294
Closed

Model.load_from_checkpoint tries to open file path as URL and fail #2243

Molaire opened this issue Jun 18, 2020 · 7 comments · Fixed by #2244 or #2294
Labels
bug Something isn't working help wanted Open to be worked on

Comments

@Molaire
Copy link
Contributor

Molaire commented Jun 18, 2020

🐛 Bug

load_from_checkpoint tries to classify if the input is a file path or an URL and detects the hard drive letter as a scheme and then classify wrongly the input because of this.
urllib.error.URLError: <urlopen error unknown url type: d>

My input:
D:\Prog\Projects\AceriNet\research_seed\checkpoints\acerinet\bnacerinet0_target=OVA_OK_penalized=None_loss_fn=ce_normalized=True_balanced=FalseFalse_seed=42_val_loss=0.374_val_auroc=0.9041_v0.ckpt

To Reproduce

Steps to reproduce the behavior:
Use any path with a hard drive letter at the start (windows formatting) for pl.LightningModule().load_from_checkpoint(path)

@Molaire Molaire added bug Something isn't working help wanted Open to be worked on labels Jun 18, 2020
@github-actions
Copy link
Contributor

Hi! thanks for your contribution!, great first issue!

@yukw777
Copy link
Contributor

yukw777 commented Jun 18, 2020

Ah I didn’t think about Windows when I implemented this. I’ll take a look if there’s a better way.

@Molaire
Copy link
Contributor Author

Molaire commented Jun 18, 2020

We could try to detect if the file exists with if os.path.isfile() or Path(path_of_chkpt).is_file() instead of checking for scheme

@awaelchli
Copy link
Contributor

Also came across this when I was running tests on windows. I wonder why it was not showing up in CI.

@Molaire
Copy link
Contributor Author

Molaire commented Jun 18, 2020

The only reason I see is that tmpdir is a relative path without prefix at the start, dodging the problem. It should be fixed by my PR, I hope.

@awaelchli
Copy link
Contributor

Thanks @Molaire. I can verify the tests now pass locally on windows.

@airium
Copy link
Contributor

airium commented Jun 19, 2020

I am afraid the PR #2244 introduced a new bug, as now on Windows Path(path_or_url).is_file() will block every URL by raising OSError. I will submit a new PR to better fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Open to be worked on
Projects
None yet
4 participants