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

Bug in evaluation script during training #252

Closed
KevinEloff opened this issue Oct 13, 2023 · 1 comment · Fixed by #253
Closed

Bug in evaluation script during training #252

KevinEloff opened this issue Oct 13, 2023 · 1 comment · Fixed by #253
Labels
bug Something isn't working

Comments

@KevinEloff
Copy link

There are some inconsistencies in how arguments are passed to the various evaluation functions. This might be causing amino acid precision and recall to be switched.

In the validation step, the peptide precision is calculated as follows:

        aa_precision, _, pep_precision = evaluate.aa_match_metrics(
            *evaluate.aa_match_batch(
                peptides_pred, peptides_true, self.decoder._peptide_mass.masses
            )
        )

The evaluate.aa_match_metrics expects the arguments in the order of first n_aa_true and then n_aa_pred.

def aa_match_metrics(
    aa_matches_batch: List[Tuple[np.ndarray, bool]],
    n_aa_true: int,
    n_aa_pred: int,
) -> Tuple[float, float, float]:

The evaluate.aa_match_batch function returns n_aa1 and n_aa2 in the same order as peptides1 and peptides2

def aa_match_batch(
    peptides1: Iterable,
    peptides2: Iterable,
    aa_dict: Dict[str, float],
    cum_mass_threshold: float = 0.5,
    ind_mass_threshold: float = 0.1,
    mode: str = "best",
) -> Tuple[List[Tuple[np.ndarray, bool]], int, int]:
    ...
    return aa_matches_batch, n_aa1, n_aa2

The problem comes in that in the validation step, we pass peptides1=peptides_pred and peptides2=peptides_true, thereby passing n_aa_true as the number of number of predicted amino acids, and vice-versa for n_aa_pred.

This shouldn't cause an issue to the current peptide-level precision calculation, but it may affect older versions of the codebase.

@bittremieux bittremieux added the bug Something isn't working label Oct 13, 2023
@bittremieux
Copy link
Collaborator

Thanks, excellent catch.

The good thing is that this bug only influenced the validation metrics reported by Casanovo directly (e.g. during training/validation). The results and figures included in the papers were obtained with external evaluation scripts, so normally those should still be correct.

melihyilmaz added a commit that referenced this issue Oct 24, 2023
melihyilmaz added a commit that referenced this issue Nov 2, 2023
* Remove unused custom_encoder option (#254)

* resolves issue #238: remove custom_encoder option

* fixed lint issue

* fixed lint issue

* Revert "fixed lint issue"

This reverts commit bd1366c.

* lint

* lint issue

* Consistently format changelog.

---------

Co-authored-by: Isha Gokhale <[email protected]>
Co-authored-by: Wout Bittremieux <[email protected]>

* Correctly report AA precision and recall during validation (#253)

Fixes #252.

Co-authored-by: Melih Yilmaz <[email protected]>

* Remove gradient calculation during inference  (#258)

* Remove force_grad in inference

* Upgrade required PyTorch version

* Update CHANGELOG.md

* Update CHANGELOG.md

* Fix typo in torch version

* Specify correct Pytorch version change

---------

Co-authored-by: Wout Bittremieux <[email protected]>

* Add label smoothing

* Modify config file

* Minor fix config.yaml

* Run black

* Lint casanovo.py

---------

Co-authored-by: ishagokhale <[email protected]>
Co-authored-by: Isha Gokhale <[email protected]>
Co-authored-by: Wout Bittremieux <[email protected]>
Co-authored-by: Wout Bittremieux <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants