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

Update to the deault github runner #186

Merged
merged 2 commits into from
Dec 14, 2023
Merged

Update to the deault github runner #186

merged 2 commits into from
Dec 14, 2023

Conversation

gq1
Copy link
Member

@gq1 gq1 commented Dec 12, 2023

change to the default runner for now.

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
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile 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).

@gq1
Update ci.yml
a5e6909 
change to the default runner for now.
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 12, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 473ee9f

+| ✅ 127 tests passed       |+
#| ❔  17 tests were ignored |#
!| ❗   7 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • nextflow_config - Config manifest.version should end in dev: '1.0.2'
  • pipeline_todos - TODO string in methods_description_template.yml: ## Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • system_exit - System.exit in WorkflowTreeval.groovy: System.exit(1) [line 17]

❔ Tests ignored:

  • files_exist - File is ignored: assets/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: conf/test_full.config
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_dark.png
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File does not exist: assets/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_dark.png
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/treeval/treeval/.github/workflows/awstest.yml

✅ Tests passed:

Run details

  • nf-core/tools version 2.8
  • Run at 2023-12-14 16:50:51

@gq1
Copy link
Member Author

gq1 commented Dec 12, 2023

@yumisims @DLBPointon
Do we have the following step in the rapid test before? I remember the rapid test can pass when using the Github default runner.
RAPID:SANGERTOL_TREEVAL_RAPID:TREEVAL_RAPID:HIC_MAPPING:JUICER_TOOLS_PRE

There is java process asking for 36G memory directly. Can we try java -Xms1g -Xmx36g?

@DLBPointon
Copy link
Contributor

Hi Guoying, sorry for taking my time getting back to you. I've been on holiday.

@yumisims @DLBPointon Do we have the following step in the rapid test before? I remember the rapid test can pass when using the Github default runner. RAPID:SANGERTOL_TREEVAL_RAPID:TREEVAL_RAPID:HIC_MAPPING:JUICER_TOOLS_PRE

The process was in there. However, it was coded to not run on the GitHub runner due to the sheer amount of resources the process needs. I thought the runner was to still get the tiny test working properly and then a large run on the farm. If not then I can add the full test back in, but Tiny will also need the custom runner.

There is java process asking for 36G memory directly. Can we try java -Xms1g -Xmx36g?

We can indeed try this.

@yumisims
Copy link
Contributor

Hi Guoying, sorry for taking my time getting back to you. I've been on holiday.

@yumisims @DLBPointon Do we have the following step in the rapid test before? I remember the rapid test can pass when using the Github default runner. RAPID:SANGERTOL_TREEVAL_RAPID:TREEVAL_RAPID:HIC_MAPPING:JUICER_TOOLS_PRE

The process was in there. However, it was coded to not run on the GitHub runner due to the sheer amount of resources the process needs. I thought the runner was to still get the tiny test working properly and then a large run on the farm. If not then I can add the full test back in, but Tiny will also need the custom runner.

There is java process asking for 36G memory directly. Can we try java -Xms1g -Xmx36g?

We can indeed try this.

@DLBPointon need Will @weaglesBio to update the juicer part first. then we could minimize the heap, I am thinking to optimise the other resource a bit, these should resolve the testing problem.

@DLBPointon
Copy link
Contributor

@DLBPointon need Will @weaglesBio to update the juicer part first. then we could minimize the heap, I am thinking to optimise the other resource a bit, these should resolve the testing problem.

Sure ok.

I'll get some decent graphs going.

@gq1 gq1 requested review from yumisims and DLBPointon December 14, 2023 17:05
yumisims
yumisims approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@yumisims yumisims left a comment

Choose a reason for hiding this comment

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

rapid testing passed, we will open another branch to carry out full test. Thanks @gq1 @DLBPointon @muffato

@yumisims yumisims merged commit 4034ce8 into dev Dec 14, 2023
6 checks passed
@gq1 gq1 deleted the change_runner_to_default branch December 14, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yumisims yumisims yumisims approved these changes

@DLBPointon DLBPointon Awaiting requested review from DLBPointon

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

Successfully merging this pull request may close these issues.
3 participants
@gq1 @DLBPointon @yumisims