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

Discussion - ANIm files using pyani v0.2 vs 0.3 differ #109

Closed
wants to merge 4 commits into from

Conversation

peterjc
Copy link
Collaborator

@peterjc peterjc commented Oct 8, 2024

For issue #102 (groundtruth ANIb using pyani v0.2, see PR #108) we need to use pyANI v0.2 rather than v0.3 when we generate the expected matrices. I thought it would be nicer therefore to also use v0.2 for generating the ANIm groundtruth values - assuming they would be the same.

The script to do this for ANIm was essentially a copy of the proposed ANIb script with a search-and-replace.

This PR is to demonstrate changes in the ANIm output from v0.2 (specifically the version_0_2 branch as of 4c19123c67b78c01d58869527402aec518ead473 to v0.3 (specifically the main branch as of 6b8b262b028f7d27c2dfdddbeda67e7cb571fee7).

First there is a trivial difference in the row/col labels switching order from v0.2 to v0.3. Therefore this PR explicitly sorts them matrix by the MD5 (as being done anyway in this repo internally as part of testing against the expected matrices).

The final commit dc87d7e on this PR shows meaningful differences including:

  • Alignment lengths were floats (ending .0) under v0.2, and included some longer than the shorter sequence (!)
  • Some coverage values of approx 1.5 (see above)
  • Knock on changes to the Hadamard
  • Sim-errors diagonals were floats (ending .0) under v0.2 with a zero diagonal, but have got diagonal values of 1 under v0.3 (!)

i.e.

  • This looks like a bug in v0.2 (overly long alignments) which was fixed in v0.3.
  • There looks to be a quirk in v0.3 with non-zero self-vs-self values of one for sim-errors

This should probably be done as a separate Python script
but this is a proof of principle for now.
This is the same script as for ANIb which has to use
v0.2 since currently the method has not been finished
in v0.3
Note we have explicitly sorted the rows/cols by MD5 as
otherwise the order flipped from pyani v0.2 to v0.3

There are still differences...
Copy link

codacy-production bot commented Oct 8, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.56% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (7e41764) 448 433 96.65%
Head commit (dc87d7e) 896 (+448) 871 (+438) 97.21% (+0.56%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#109) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Oct 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

see 1 file with indirect coverage changes

@peterjc peterjc changed the title Dsicussion - ANIm files using pyani v0.2 vs 0.3 differ Discussion - ANIm files using pyani v0.2 vs 0.3 differ Oct 8, 2024
@peterjc
Copy link
Collaborator Author

peterjc commented Oct 8, 2024

(This test will pass right now as we've not yet merged #107 which will check the pyANI plus ANIm calculations against these expected values)

@peterjc
Copy link
Collaborator Author

peterjc commented Oct 8, 2024

Confirmed with @kiepczi this is due to the bug in v0.2 fixed in v0.3 by widdowquinn/pyani#425

This means we will need to split the makefile fixtures entry as we need both pyani 0.2 (for ANIb) and 0.3 (for ANIm).

See also 7e41764 which I made to stop accidentally using the wrong pyani - similar guard in #108.

@peterjc
Copy link
Collaborator Author

peterjc commented Oct 8, 2024

Closing in favour of issue widdowquinn/pyani#438

@peterjc peterjc closed this Oct 8, 2024
peterjc added a commit that referenced this pull request Oct 8, 2024
See discussion on #109, we need different versions of pyani
for ANIm (v0.3) and ANIb (v0.2).
@peterjc peterjc deleted the regenerate_anim branch October 8, 2024 16:13
peterjc added a commit that referenced this pull request Oct 10, 2024
See discussion on #109, we need different versions of pyani
for ANIm (v0.3) and ANIb (v0.2).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant