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

Eliminate the eval command #355

Closed
wsnoble opened this issue Jul 8, 2024 · 4 comments · Fixed by #359
Closed

Eliminate the eval command #355

wsnoble opened this issue Jul 8, 2024 · 4 comments · Fixed by #359
Assignees
Labels
enhancement New feature or request

Comments

@wsnoble
Copy link
Contributor

wsnoble commented Jul 8, 2024

The eval command is just like the sequence command except that it requires annotated spectra. We should get rid of it, and then have the program automatically do evaluations if the user provides annotated spectra. If no validation set is provided, the evaluation should be done on the training set. Results can be printed to the log file, as is done currently.

@bittremieux bittremieux added the enhancement New feature or request label Jul 9, 2024
@bittremieux
Copy link
Collaborator

bittremieux commented Jul 9, 2024

have the program automatically do evaluations if the user provides annotated spectra

This is a bit tricky, because you'd need to inspect the file first to check whether there are peptide sequences included in order to know how to instantiate the data loader (annotated MGF or standard MGF).

Easier would be the alternative option that adds an --evaluate flag.

@wsnoble
Copy link
Contributor Author

wsnoble commented Jul 9, 2024

Yes, --evaluate is fine with me.

@Lilferrit
Copy link
Contributor

Lilferrit commented Jul 11, 2024

Do we still want to output the sequence results file if the evaluate flag is set? If so I think a good apprach might be to add a boolean argument to _predict_impl (and predict) that runs validation if set to true, and then we can get rid of the _validate_impl and validate functions from the trainer all-together. This boolean flag would then just be passed from the command line down into Trainer._predict_impl.

Edit: Nevermind, I followed the function calls all the way into the PyLightning library without realizing it. I'll think of another approach.

@wsnoble
Copy link
Contributor Author

wsnoble commented Jul 11, 2024

Yes, we should output sequencing results if the flag is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants