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.

Sign up for GitHub

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

Jump to bottom

Fastq align dedup bwameth #7007

Merged
merged 48 commits into from
Nov 30, 2024
Merged

Fastq align dedup bwameth #7007

merged 48 commits into from
Nov 30, 2024

Conversation

sateeshperi
Copy link
Contributor

No description provided.

@sateeshperi sateeshperi self-assigned this Nov 17, 2024
@sateeshperi sateeshperi linked an issue Nov 17, 2024 that may be closed by this pull request
new subworkflow: fastq_align_dedup_bwameth #6973
Closed
4 tasks
@sateeshperi sateeshperi requested a review from a team as a code owner November 17, 2024 11:43
@sateeshperi sateeshperi mentioned this pull request Nov 18, 2024
Closed
sateeshperi and others added 15 commits November 18, 2024 09:23
adamrtalbot
adamrtalbot approved these changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@adamrtalbot adamrtalbot left a comment

Choose a reason for hiding this comment

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

All good - complexity is bordering on too much now. It's a shame we can't redirect individual tests to specific hardware, you know...like a certain piece of software does very well.

This manages most of the issues well.

.github/workflows/gpu-tests.yml Outdated Show resolved Hide resolved
@edmundmiller
Copy link
Contributor

Nextflow GitHub actions executor when?

On the complexity, after we merge this one can we migrate the action to live in this repo? I think that structure works better for the mono-repo so these PRs aren't so complicated to test/hack on.

@GallVp
Copy link
Member

GallVp commented Nov 29, 2024

Hey Team
Thanks for the reviews. A couple more things before we merge.

  1. Remove DEBUG flag
  2. Redirect back to adam/detect-nf-tes-changes after adding a new release there. I prefer that we keep the CI logic in a seperate repo. It is messy but does afford separation of concerns. Moreover, I and possibly other community members are using it for their own org level/personal modules repos. Why we have personal is another matter and happy to receive criticism on it.
  3. Add gpu-confirm-pass and put it on the required list.
  4. Cleanup/optimise the triggers under gpu-test action.

There are many outstanding issues including conda fail but I guess we can work on them after the merge as Sateesh needs this for methylseq work.

@GallVp
Copy link
Member

GallVp commented Nov 30, 2024 •
edited
Loading

Related PRs for CI evaluation,

  1. This is a dummy PR to test CI #7124; Just a random change in .prettierrc.yml.
  2. Touched a single module to check CI #7125, Touched a single CPU-based module.

@GallVp GallVp added this pull request to the merge queue Nov 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 30, 2024
@sateeshperi sateeshperi added this pull request to the merge queue Nov 30, 2024
@sateeshperi sateeshperi removed this pull request from the merge queue due to a manual request Nov 30, 2024
@sateeshperi sateeshperi merged commit bd5f75c into master Nov 30, 2024
26 of 29 checks passed
@sateeshperi sateeshperi deleted the fastq_align_dedup_bwameth branch November 30, 2024 06:29
LouisLeNezet pushed a commit to LouisLeNezet/modules that referenced this pull request Dec 4, 2024
@sateeshperi @SPPearce @GallVp
Fastq align dedup bwameth (nf-core#7007)
0a47c53 
* init bwameth subworkflow

* update output channels

* bwameth single, paired-end default, skip_dedup tests

* add GPU tests

* separate GPU tests

* add subworkflow GPU test path to test.yml include

* add subworkflow GPU test path to test.yml include

* add subworkflow GPU test path to test.yml include

* use more descriptive collect variables than it

* rename file to be explicit

* separate profile exclusions

* add gpu tag

* rm old test.yml

* add gpu test path to gpu-tests.yml:ci

* Added log_level: DEBUG

* Setup CI for debug

* add more debug steps

* usman's fix for exclude tags

* Updated fail condition

* ci sync to master

* fix filtering by usman

* Removed --changed-since when tags are supplied

* Enabled more checks and added a non-gpu module

* Two fixes

* Now using paths

* Fixed paths

* Fixed typo

* Now pass all paths instead of matrix

* Added confirm pass, cleaned dispatch and removed DEBUG

* Renamed all passes to confirm-pass

* Now using adamrtalbot/detect-nf-test-changes

* small ch format fix

---------

Co-authored-by: Simon Pearce <[email protected]>
Co-authored-by: Usman Rashid <[email protected]>
@edmundmiller edmundmiller mentioned this pull request Dec 13, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GallVp GallVp GallVp left review comments

@adamrtalbot adamrtalbot adamrtalbot approved these changes

Assignees

@sateeshperi sateeshperi

Labels
new subworkflow Ready for Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

new subworkflow: fastq_align_dedup_bwameth
5 participants
@sateeshperi @edmundmiller @GallVp @adamrtalbot @SPPearce