refactor: adjust `--cores` settings for profiles

AngryMaciek commented 1 year ago

Description

Adjusted the execution script and snakemake profiles to the following settings:

no changes for the local execution, all was fine there
if CORES is provided from the command line it overrides --cores parameter for the profile; in case of cluster execution it limits the resources snakemake will use on the cluster (even though it reserves more, as you checked); therefore it makes sense to skip this flag for cluster execution because:
the default values for CORES is now all, and it also gets passed to snakemake argument --cores; in such case the process will use as many CPUs as requested.
the settings above make sense in case of local execution, though, where the resources should be limmitted from the command line
jobs param removed completely from the profiles, not necessary

See more info from the docs:

@ajgruber : please take a look at this before our next meeting, if everything is as you wished I will merge this on Tuesday.

Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation updated
[x] Codebase refactoring

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] My changes generate no new warnings

AngryMaciek commented 1 year ago

CC @Iankouzel : I heard you were fighting with this issue, see above for clarification.

IanKouzel commented 1 year ago

@AngryMaciek thanks, good adjustment

IanKouzel commented 1 year ago

@AngryMaciek in execution/run.sh it is stated that "Singularity technology is not supported for Grid Engine yet". It is just not implemented for MAPP? Are you going to include it at some point?

AngryMaciek commented 1 year ago

@IanKouzel : The profile is ready but as we tested it a few weeks ago we got an error, we dropped it there as I have other matters to attend to now. See #15

Yes, I think I will include it at some point. After we submit the manuscript.

AngryMaciek commented 1 year ago

@ajgruber @IanKouzel : My bad, jobs is required, as I just learned during final testing before merge ( if your login node crashes today that's me :P )

Error: you need to specify the maximum number of jobs to be queued or executed at the same time with --jobs or -j.

I will put 4096, should not be limmitting us ever

gruber-sciencelab / MAPP