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

Evaluation over the validation set #4634

Closed
EliaCereda opened this issue Nov 12, 2020 · 10 comments · Fixed by #4948
Closed

Evaluation over the validation set #4634

EliaCereda opened this issue Nov 12, 2020 · 10 comments · Fixed by #4948
Assignees
Labels
design Includes a design discussion feature Is an improvement or enhancement refactor
Milestone

Comments

@EliaCereda
Copy link
Contributor

What is the recommended way of performing just one evaluation over the validation set? Basically, I'm looking for the equivalent of trainer.test(...) but for the validation set.

Maybe this is possible using trainer.fit(...) and some combination of arguments to Trainer.__init__?

@SkafteNicki
Copy link
Member

Is there a difference between your validation_step and test_step? If not you could just feed in the validation set to trainer.test(test_dataloaders=val_dataloaders).

@EliaCereda
Copy link
Contributor Author

Yes, this would be acceptable, but I forgot to specify that I'm using a data module to manage the data loaders. There is no way to pass the data module but override which loader to use, am I wrong?

The solution I'll attempt right now is to copy the trainer.test(...) method in my code and modify it to use the validation loader and validation_step. Let me know if you have a better idea.

@rohitgr7
Copy link
Contributor

rohitgr7 commented Nov 12, 2020

should we add an evaluate method too exactly similar to test? recently got same request on slack too.

@EliaCereda
Copy link
Contributor Author

It sounds good to me. I'm looking at the code of trainer.test(...) right now and it looks like this isn't something I can easily add from my own code: the interesting part is inside the fit(...) method, triggered by self.testing = True.

@SkafteNicki
Copy link
Member

@rohitgr7 sounds like a good enhancement (especially if multiple people are requesting it). I see two options:

  1. add a new method: i propose to call it validate that works similar to test
  2. add an extra argument to test if it should use test_dataloader or validation_dataloader

@rohitgr7
Copy link
Contributor

add an extra argument to test if it should use test_dataloader or validation_dataloader

thought this too.. but somehow sounds a bit complex to me.

add a new method: i propose to call it validate that works similar to test

+1, refactor the .test method to avoid duplicate code and reuse in .validate

@EliaCereda
Copy link
Contributor Author

+1 for option 1 from me too

@rohitgr7
Copy link
Contributor

cc @PyTorchLightning/core-contributors

@SkafteNicki SkafteNicki added feature Is an improvement or enhancement refactor labels Nov 12, 2020
@SkafteNicki SkafteNicki added this to the 1.1 milestone Nov 12, 2020
@EliaCereda
Copy link
Contributor Author

I've been experimenting with the changes required to make this work and I just published #4707 with a very rough proof of concept of what will need to be done.

@0phoff
Copy link

0phoff commented Dec 21, 2020

I know there will be a validate() method which will simplify this, but in the meantime, here is how I got things working in PL v1.1.1:

# Create modules
model = pl.LightningModule(...)
dm = pl.LightningDataModule(...)

# Manually run prep methods on DataModule
dm.prepare_data()
dm.setup()

# Run test on validation dataset
train = pl.Trainer(...)
train.test(model, test_dataloaders=dm.val_dataloader())

@Borda Borda modified the milestones: 1.2, 1.2.x Feb 18, 2021
@edenlightning edenlightning modified the milestones: 1.2.x, 1.3 Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Includes a design discussion feature Is an improvement or enhancement refactor
Projects
None yet
7 participants