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

fix: update and rearrange munin configuration file and docs #65

Merged
merged 7 commits into from Aug 16, 2019

Conversation

maxulysse
Copy link
Member

  • Improve description
  • Add maxForks=48 in process
  • Update max_memory to 754.GB
  • Update max_cpus to 48
  • Fix path to iGenomes

@maxulysse maxulysse requested review from szilvajuhos and a team August 7, 2019 09:13
@maxulysse
Copy link
Member Author

I closed #52, because, I totally forgot it was there, so I deleted my fork before I noticed I wanted to do some changes on this PR...

Copy link
Member

@alneberg alneberg left a comment

Choose a reason for hiding this comment

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

No problems what I can see.

conf/munin.config Outdated Show resolved Hide resolved
conf/munin.config Outdated Show resolved Hide resolved
docs/munin.md Outdated
To use, run the pipeline with `-profile munin`. This will download and launch the [`munin.config`](../conf/munin.config) which has been pre-configured with a setup suitable for the MUNIN cluster. Using this profile, a docker image containing all of the required software will be downloaded, and converted to a Singularity image before execution of the pipeline.
To use, run the pipeline with `-profile munin`.
This will download and launch the [`munin.config`](../conf/munin.config) which has been pre-configured with a setup suitable for the MUNIN cluster.
Using this profile, a docker image containing all of the required software will be downloaded, and converted to a Singularity image before execution of the pipeline.
Copy link
Member

Choose a reason for hiding this comment

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

You can run with either docker or singularity with this profile? May need to be clarified here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can, but good point, I'll clarify that

Copy link
Member Author

Choose a reason for hiding this comment

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

Made some clarification

maxulysse and others added 2 commits August 7, 2019 11:47
Co-Authored-By: Harshil Patel <drpatelh@users.noreply.github.com>
Co-Authored-By: Harshil Patel <drpatelh@users.noreply.github.com>
@matq007
Copy link
Member

matq007 commented Aug 7, 2019

Wow, are we sure we want to set the max memory that high? Same for CPU, the linux itself needs something to run on :D

@maxulysse
Copy link
Member Author

Yes, VEP is very memory hungry...
But @szilvajuhos can maybe confirm on that matter

@maxulysse
Copy link
Member Author

I also removed autoMounts which was definitively failing when I tried

@matq007
Copy link
Member

matq007 commented Aug 7, 2019

Well at least reserve 2GB and 2 CPU just for the linux. I mean if you force it to much the server will crash or your VEP will throw segfault.

@maxulysse
Copy link
Member Author

That does sound sensible :-D

@drpatelh
Copy link
Member

drpatelh commented Aug 7, 2019

Ahhh...so nice to see the tests passing again 😅

Copy link

@szilvajuhos szilvajuhos left a comment

Choose a reason for hiding this comment

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

Looks good, just trying out.

@szilvajuhos szilvajuhos merged commit e5d8f20 into nf-core:master Aug 16, 2019
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.

None yet

5 participants