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

port parabricks/deepvariant to nf-test #6995

Merged
merged 33 commits into from
Jan 20, 2025
Merged

Conversation

famosab
Copy link
Contributor

@famosab famosab commented Nov 14, 2024

PR checklist

Closes #XXX

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

@famosab famosab requested a review from a team as a code owner November 14, 2024 14:31
@famosab famosab requested a review from mirpedrol November 14, 2024 14:31
@famosab
Copy link
Contributor Author

famosab commented Nov 14, 2024

Currently assessing the created g.vcf always fails with

╭────────────────────────────────────────────────────────────────────────────────────────────────────────────── nf-test error ───────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ groovy.lang.MissingMethodException: No signature of method: static com.askimed.nf.test.lang.extensions.GlobalMethods.file() is applicable for argument types: (ArrayList) values: [[/home/paifb01/modules/.nf-test/tests/2608642d653b0463a │
│ Possible solutions: file(java.lang.String), find(), find(groovy.lang.Closure), use([Ljava.lang.Object;), with(groovy.lang.Closure), is(java.lang.Object)                                                                                   │
│                                                                                                                                                                                                                                            │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

@famosab famosab added gpu module uses GPU nf-test help wanted Extra attention is needed labels Nov 18, 2024
@famosab famosab added Ready for Review and removed help wanted Extra attention is needed labels Dec 16, 2024
@famosab
Copy link
Contributor Author

famosab commented Jan 15, 2025

Now the runners are picking up and some of the tests pass but some do not. Locally everything looks fine:

 nf-core modules test parabricks/deepvariant -u --profile docker


                                          ,--./,-.
          ___     __   __   __   ___     /,-._.--~\ 
    |\ | |__  __ /  ` /  \ |__) |__         }  {
    | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                          `._,._,'

    nf-core/tools version 3.0.2 - https://nf-co.re
    There is a new version of nf-core/tools available! (3.1.1)


INFO     Generating nf-test snapshot                                                                                                                                            
╭─────────────────────────────────────────────────────────────────────────────── nf-test output ───────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                              │
│ 🚀 nf-test 0.9.2                                                                                                                                                             │
│ https://www.nf-test.com                                                                                                                                                      │
│ (c) 2021 - 2024 Lukas Forer and Sebastian Schoenherr                                                                                                                         │
│                                                                                                                                                                              │
│ Load .nf-test/plugins/nft-bam/0.5.0/nft-bam-0.5.0.jar                                                                                                                        │
│ Load .nf-test/plugins/nft-compress/0.1.0/nft-compress-0.1.0.jar                                                                                                              │
│ Load .nf-test/plugins/nft-vcf/1.0.7/nft-vcf-1.0.7.jar                                                                                                                        │
│ Warning: every snapshot that fails during this test run is re-record.                                                                                                        │
│                                                                                                                                                                              │
│ Test Process PARABRICKS_DEEPVARIANT                                                                                                                                          │
│                                                                                                                                                                              │
│   Test [d4150cc7] 'human - bam' PASSED (19.026s)                                                                                                                             │
│   Test [7915f0a7] 'human - bam - intervals' PASSED (18.794s)                                                                                                                 │
│   Test [4a9986c1] 'human - bam - gvcf' PASSED (22.216s)                                                                                                                      │
│   Test [2608642d] 'human - bam - intervals - gvcf' PASSED (17.976s)                                                                                                          │
│   Test [92648df2] 'human - bam - stub' PASSED (10.546s)                                                                                                                      │
│   Test [cd720ada] 'human - bam - intervals - gvcf - stub' PASSED (13.86s)                                                                                                    │
│                                                                                                                                                                              │
│                                                                                                                                                                              │
│ SUCCESS: Executed 6 tests in 102.508s                                                                                                                                        │
│                                                                                                                                                                              │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
INFO     Generating nf-test snapshot again to check stability                                                                                                                   
╭─────────────────────────────────────────────────────────────────────────────── nf-test output ───────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                              │
│ 🚀 nf-test 0.9.2                                                                                                                                                             │
│ https://www.nf-test.com                                                                                                                                                      │
│ (c) 2021 - 2024 Lukas Forer and Sebastian Schoenherr                                                                                                                         │
│                                                                                                                                                                              │
│ Load .nf-test/plugins/nft-bam/0.5.0/nft-bam-0.5.0.jar                                                                                                                        │
│ Load .nf-test/plugins/nft-compress/0.1.0/nft-compress-0.1.0.jar                                                                                                              │
│ Load .nf-test/plugins/nft-vcf/1.0.7/nft-vcf-1.0.7.jar                                                                                                                        │
│                                                                                                                                                                              │
│ Test Process PARABRICKS_DEEPVARIANT                                                                                                                                          │
│                                                                                                                                                                              │
│   Test [d4150cc7] 'human - bam' PASSED (20.539s)                                                                                                                             │
│   Test [7915f0a7] 'human - bam - intervals' PASSED (17.475s)                                                                                                                 │
│   Test [4a9986c1] 'human - bam - gvcf' PASSED (16.655s)                                                                                                                      │
│   Test [2608642d] 'human - bam - intervals - gvcf' PASSED (18.481s)                                                                                                          │
│   Test [92648df2] 'human - bam - stub' PASSED (10.007s)                                                                                                                      │
│   Test [cd720ada] 'human - bam - intervals - gvcf - stub' PASSED (9.339s)                                                                                                    │
│                                                                                                                                                                              │
│                                                                                                                                                                              │
│ SUCCESS: Executed 6 tests in 92.639s                                                                                                                                         │
│                                                                                                                                                                              │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
INFO     All tests passed! 

@SPPearce
Copy link
Contributor

The runners seem to be giving a different md5sum for the path(process.out.vcf[0][1]).vcf.variantsMD5

@SPPearce
Copy link
Contributor

I would wonder if the variants are in a different order, can you just print all the lines to the snapshot to see what is different.

@famosab
Copy link
Contributor Author

famosab commented Jan 15, 2025

@SPPearce Changed it to assert the first 100 variants (which are more than are contained). Do you think thats ok?

@SPPearce
Copy link
Contributor

Sure, its just to try and debug the issue

@famosab
Copy link
Contributor Author

famosab commented Jan 15, 2025

Now only the linting fails - so I am not sure why the md5sums are different on the runners and locally.

Copy link
Contributor

@SPPearce SPPearce left a comment

Choose a reason for hiding this comment

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

Ok, tests are passing!
It would be nice to gzip the output for space reasons, but I'm not going to argue that.

@famosab
Copy link
Contributor Author

famosab commented Jan 16, 2025

Lets get this merged then for now :) I will put in a request with the parabricks people to enable bgzip.

@famosab famosab added this pull request to the merge queue Jan 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 16, 2025
@SPPearce SPPearce added this pull request to the merge queue Jan 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 16, 2025
@SPPearce SPPearce added this pull request to the merge queue Jan 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 16, 2025
@famosab famosab added this pull request to the merge queue Jan 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 17, 2025
@famosab famosab added this pull request to the merge queue Jan 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 17, 2025
@famosab famosab added this pull request to the merge queue Jan 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 17, 2025
@sateeshperi sateeshperi merged commit a313e38 into nf-core:master Jan 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants