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

[caprieval] comparisons against multiple references #1136

Open
wants to merge 4 commits into
base: multi-ref-caprieval
Choose a base branch
from

Conversation

VGPReys
Copy link
Contributor

@VGPReys VGPReys commented Nov 8, 2024

Checklist

  • Tests added for the new code
  • Documentation added for the code changes
  • Does not break licensing
  • Does not add any dependencies, if it does please add a thorough explanation

Summary of the Pull Request

This PR in made to handle comparisons with multiple references.

This is performed by turning on the parameter multiple_references = true (default false).
In this scenario, reported values in capri_ss.tsv are the ones obtained against the best reference.

If multiple_references = false (default), the first reference will be used as reference.

I had to add the __eq__ and __lt__ methods to the CAPRI class, so comparisons could be performed by functions/methods such as sorted([CAPRI, CAPRI]) or [CAPRI, CAPRI].sort().

Related Issue

#1133

@VGPReys VGPReys added enhancement Enhancing an existing feature of adding a new one feature New feature request m|caprieval Improvements in caprieval module labels Nov 8, 2024
@VGPReys VGPReys requested a review from mgiulini November 8, 2024 12:00
@VGPReys VGPReys self-assigned this Nov 8, 2024
Comment on lines +17 to +21
[topoaa]

[caprieval]
# Comparison agains first reference
reference_fname = "data/ensemble_4G6M.pdb"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this example confusing for users: why using the same input ensemble as a reference in caprieval? the fact that we support this does not mean we should encourage this..this could become an integration test!

Copy link
Contributor Author

@VGPReys VGPReys Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed !

At least we see that we obtain 1 in dockq and 0 in rmsd.
And clearly highlighting the behavior of this approach.

But I think that indeed this is a very bad example that we do not want to showcase.
I will find an other reference file !

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can also remove the example and we only have this as integration test..depends on how much we want to advertise this feature

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment on lines +11 to +21
multiple_references:
default: false
type: boolean
title: Allow the use of multiple reference conformations.
short: Performs caprieval computation on multiple reference conformations.
long: Performs caprieval computation on multiple reference conformations.
Final values will contain the best performing metrics with respect to
all reference conformations. Usefull when evaluating docking poses against
NMR structures, for example.
group: analysis
explevel: expert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this parameter? isn't it easier to evaluate the quality of whatever is provided as reference_fname in caprieval? maybe limiting the number of reference structures to 10 or 20

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make sure the user is willing to compare against multiple references.
Could be removed and always applied.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove it..I think the logic gets too complicated otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancing an existing feature of adding a new one feature New feature request m|caprieval Improvements in caprieval module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants