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

Add profile for ARM compatibility #1425

Merged
merged 49 commits into from
Oct 23, 2024
Merged

Add profile for ARM compatibility #1425

merged 49 commits into from
Oct 23, 2024

Conversation

pinin4fjords
Copy link
Member

@pinin4fjords pinin4fjords commented Oct 22, 2024 •
edited
Loading

  1. Add a config profile for ARM, referencing ARM-specific containers
  2. Update the Trim Galore module with a new conda build with an ARM-compatible cutadapt build

PR checklist

  • 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 pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/rnaseq branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

pabloaledo and others added 23 commits October 17, 2024 09:26
@pabloaledo
appy changes for ARM
d59ecf7 
Signed-off-by: Pablo <[email protected]>
@pinin4fjords
Merge branch 'arm_3.16.1' into dev
991637c
@pinin4fjords
Fix up config
4bdae48
@pinin4fjords
Reset environment ymls
c9d616b
@pinin4fjords
Reset main.nfs with software changes only
9f640ab
@pinin4fjords
Link sw configs to arm profile
00cff4c
@pinin4fjords
Add process block
f281e0b
@pinin4fjords
note pins
f5208dd
@pinin4fjords
Remove conda overrides that were having no impact
ea8b9e8
@pinin4fjords
Use different RSEQC pin
db02d99
@pinin4fjords
update BEDGRAPHTOBIGWIG pin
4a8ed2b
@pinin4fjords
Fix annotation
b86a6f9
@pinin4fjords
Don't think we need the RSEQC pin
1b96eb8
@pinin4fjords
correct ucsc
4e5d7a3
@pabloaledo
Merge pull request #1 from pinin4fjords/sw_to_conf
bbd3452 
Sw to conf
@pinin4fjords
Triage arm deps
902367e
@pinin4fjords
Non-igenomes STAR can use the latest star
328d02d
@pinin4fjords
remove rogue comment
2f92bfc
@pinin4fjords
update arm conf
0a563dc
@pinin4fjords
Cut out module redundant software overrides
0be2d6a
@pabloaledo
Merge pull request #2 from pinin4fjords/triage_arm
bba6b92 
Triage arm deps
@pinin4fjords
Merge pull request #1414 from pabloaledo/dev
56f1a47 
Backport support for Graviton 4 to version 3.16.1
@pinin4fjords
Merge branch 'dev' into arm_3.16.1
a5e831f
@pinin4fjords pinin4fjords mentioned this pull request Oct 22, 2024
Merged
11 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 22, 2024 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 424137a

+| ✅ 186 tests passed       |+
#| ❔   9 tests were ignored |#
!| ❗   8 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: assets/multiqc_config.yml
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!
  • pipeline_todos - TODO string in base.config: Check the defaults for all processes
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your preferred methods description, e.g. add publication citation for this pipeline

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-10-23 12:56:09

maxulysse
maxulysse reviewed Oct 22, 2024
View reviewed changes
Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

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

Looking good. How did you generate the config file?
Nextflow inspect?
Do we test for this anywhere?

drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
docs/usage.md Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
conf/arm.config Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
conf/arm.config Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
conf/arm.config Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
conf/arm.config Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
conf/arm.config Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
docs/usage.md Outdated
-profile docker,arm
```

This will use ARM-compatible containers, and apply a small number of overrides to Conda definitions to support ARM operation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This will use ARM-compatible containers, and apply a small number of overrides to Conda definitions to support ARM operation.
This will use ARM-compatible containers, and apply a small number of overrides to Conda definitions to support ARM operations.

drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
docs/usage.md Outdated Show resolved Hide resolved
drpatelh
drpatelh reviewed Oct 23, 2024
View reviewed changes
docs/usage.md Outdated
Comment on lines 454 to 457
:::warning
Please note that the ARM profile is experimental. It is expected to function correctly in all cases unless explicitly indicated otherwise—currently, exceptions include the use of the hisat2 aligner and contaminant screening via kraken2. However, because testing is presently conducted manually, we cannot guarantee its reliability.
:::

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:::warning
Please note that the ARM profile is experimental. It is expected to function correctly in all cases unless explicitly indicated otherwise—currently, exceptions include the use of the hisat2 aligner and contaminant screening via kraken2. However, because testing is presently conducted manually, we cannot guarantee its reliability.
:::

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we just link to the new section we created in the arm profile above? Means we don't need to repeat the text.

drpatelh
drpatelh approved these changes Oct 23, 2024
View reviewed changes
Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@pinin4fjords
Copy link
Member Author

Thanks @drpatelh !

@pinin4fjords pinin4fjords merged commit fb057c9 into dev Oct 23, 2024
32 checks passed
@pinin4fjords pinin4fjords deleted the arm_3.16.1 branch October 23, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxulysse maxulysse maxulysse left review comments

@pditommaso pditommaso pditommaso left review comments

@drpatelh drpatelh drpatelh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pinin4fjords @pditommaso @maxulysse @drpatelh @pabloaledo