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

Replace check_max function with resourceLimits directive #3037

Merged
merged 22 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ee2241a
Replace check_max with resourceLimits
jfy133 Jun 25, 2024
ce49bad
Update CHANGELOG.md
jfy133 Jun 25, 2024
c8abbef
Code alignment, and try to get test to run on GHA Actions runner
jfy133 Jun 25, 2024
044fc1d
Remove default basic resource limits in template (should be set by us…
jfy133 Jul 1, 2024
528a207
Add a basic set of resource limits in test configs, matching GithubAc…
jfy133 Jul 1, 2024
936802f
Bump minimum NXF version to allow resourceLimits
jfy133 Jul 1, 2024
06a1dbe
Add linting check for now deprecated params
jfy133 Jul 1, 2024
47abfc2
Add linting check for check_ params and continue removing from everyw…
jfy133 Jul 1, 2024
2e690d9
Merge branch 'dev' into deprecate-cehck_max_function
jfy133 Sep 4, 2024
736d5e8
Removes all remaining references to max_* params
jfy133 Sep 4, 2024
5563360
[automated] Fix code linting
nf-core-bot Sep 4, 2024
6633769
Fix linting fail
jfy133 Sep 4, 2024
d819232
Fix schema error
jfy133 Sep 4, 2024
f3138fc
Merge branch 'deprecate-cehck_max_function' of github.com:jfy133/nf-c…
jfy133 Sep 4, 2024
5599039
add tests back
mirpedrol Sep 16, 2024
e87478d
Merge branch 'dev' of https://github.com/nf-core/tools into deprecate…
mirpedrol Sep 16, 2024
c568ce9
more pytest fixing
mirpedrol Sep 16, 2024
17dc0bc
change order of profiles used for CI
mirpedrol Sep 19, 2024
482c40b
Merge branch 'dev' into deprecate-cehck_max_function
mirpedrol Sep 19, 2024
c494c26
update nextflow version on CI
mirpedrol Sep 19, 2024
2c0d2f6
configs are included after profiles
mirpedrol Sep 19, 2024
d2959ed
reduce memory in test profile
mirpedrol Sep 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
- add option to exclude test configs from pipeline template ([#3133](https://github.com/nf-core/tools/pull/3133))
- add option to exclude tower.yml from pipeline template ([#3134](https://github.com/nf-core/tools/pull/3134))
- Add tests to ensure all files are part of a template customisation group and all groups are tested ([#3099](https://github.com/nf-core/tools/pull/3099))
- Replaces the old custom `check_max()` function with the Nextflow native `resourceLimits` directive ([#3037](https://github.com/nf-core/tools/pull/3037))

### Linting

Expand Down
34 changes: 17 additions & 17 deletions nf_core/pipeline-template/conf/base.config
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
process {

// TODO nf-core: Check the defaults for all processes
cpus = { check_max( 1 * task.attempt, 'cpus' ) }
memory = { check_max( 6.GB * task.attempt, 'memory' ) }
time = { check_max( 4.h * task.attempt, 'time' ) }
cpus = { 1 * task.attempt }
memory = { 6.GB * task.attempt }
time = { 4.h * task.attempt }

errorStrategy = { task.exitStatus in ((130..145) + 104) ? 'retry' : 'finish' }
maxRetries = 1
Expand All @@ -27,30 +27,30 @@ process {
// TODO nf-core: Customise requirements for specific processes.
// See https://www.nextflow.io/docs/latest/config.html#config-process-selectors
withLabel:process_single {
cpus = { check_max( 1 , 'cpus' ) }
memory = { check_max( 6.GB * task.attempt, 'memory' ) }
time = { check_max( 4.h * task.attempt, 'time' ) }
cpus = { 1 }
memory = { 6.GB * task.attempt }
time = { 4.h * task.attempt }
}
withLabel:process_low {
cpus = { check_max( 2 * task.attempt, 'cpus' ) }
memory = { check_max( 12.GB * task.attempt, 'memory' ) }
time = { check_max( 4.h * task.attempt, 'time' ) }
cpus = { 2 * task.attempt }
memory = { 12.GB * task.attempt }
time = { 4.h * task.attempt }
}
withLabel:process_medium {
cpus = { check_max( 6 * task.attempt, 'cpus' ) }
memory = { check_max( 36.GB * task.attempt, 'memory' ) }
time = { check_max( 8.h * task.attempt, 'time' ) }
cpus = { 6 * task.attempt }
memory = { 36.GB * task.attempt }
time = { 8.h * task.attempt }
}
withLabel:process_high {
cpus = { check_max( 12 * task.attempt, 'cpus' ) }
memory = { check_max( 72.GB * task.attempt, 'memory' ) }
time = { check_max( 16.h * task.attempt, 'time' ) }
cpus = { 12 * task.attempt }
memory = { 72.GB * task.attempt }
time = { 16.h * task.attempt }
}
withLabel:process_long {
time = { check_max( 20.h * task.attempt, 'time' ) }
time = { 20.h * task.attempt }
}
withLabel:process_high_memory {
memory = { check_max( 200.GB * task.attempt, 'memory' ) }
memory = { 200.GB * task.attempt }
}
withLabel:error_ignore {
errorStrategy = 'ignore'
Expand Down
13 changes: 8 additions & 5 deletions nf_core/pipeline-template/conf/test.config
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,18 @@
----------------------------------------------------------------------------------------
*/

process {
resourceLimits = [
cpus: 4,
memory: '16.GB',
time: '1.h'
]
}

params {
config_profile_name = 'Test profile'
config_profile_description = 'Minimal test dataset to check pipeline function'

// Limit resources so that this can run on GitHub Actions
max_cpus = 2
max_memory = '6.GB'
max_time = '6.h'

// Input data
// TODO nf-core: Specify the paths to your test data on nf-core/test-datasets
// TODO nf-core: Give any required params for the test so that command line flags are not needed
Expand Down
40 changes: 1 addition & 39 deletions nf_core/pipeline-template/nextflow.config
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ params {
config_profile_url = null
{%- endif %}

// Max resource options
// Defaults only, expecting to be overwritten
max_memory = '128.GB'
max_cpus = 16
max_time = '240.h'

{%- if nf_schema %}
// Schema validation default options
validate_params = true
Expand Down Expand Up @@ -271,7 +265,7 @@ manifest {
homePage = 'https://github.com/{{ name }}'
description = """{{ description }}"""
mainScript = 'main.nf'
nextflowVersion = '!>=23.10.0'
nextflowVersion = '!>=24.04.2'
version = '{{ version }}'
doi = ''
}
Expand Down Expand Up @@ -318,35 +312,3 @@ validation {
// Load modules.config for DSL2 module specific options
includeConfig 'conf/modules.config'
{% endif %}
// Function to ensure that resource requirements don't go beyond
// a maximum limit
def check_max(obj, type) {
if (type == 'memory') {
try {
if (obj.compareTo(params.max_memory as nextflow.util.MemoryUnit) == 1)
return params.max_memory as nextflow.util.MemoryUnit
else
return obj
} catch (all) {
println " ### ERROR ### Max memory '${params.max_memory}' is not valid! Using default value: $obj"
return obj
}
} else if (type == 'time') {
try {
if (obj.compareTo(params.max_time as nextflow.util.Duration) == 1)
return params.max_time as nextflow.util.Duration
else
return obj
} catch (all) {
println " ### ERROR ### Max time '${params.max_time}' is not valid! Using default value: $obj"
return obj
}
} else if (type == 'cpus') {
try {
return Math.min( obj, params.max_cpus as int )
} catch (all) {
println " ### ERROR ### Max cpus '${params.max_cpus}' is not valid! Using default value: $obj"
return obj
}
}
}
38 changes: 0 additions & 38 deletions nf_core/pipeline-template/nextflow_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,41 +136,6 @@
}
},
{%- endif %}
"max_job_request_options": {
"title": "Max job request options",
"type": "object",
"fa_icon": "fab fa-acquisitions-incorporated",
"description": "Set the top limit for requested resources for any single job.",
"help_text": "If you are running on a smaller system, a pipeline step requesting more resources than are available may cause the Nextflow to stop the run with an error. These options allow you to cap the maximum resources requested by any single job so that the pipeline will run on your system.\n\nNote that you can not _increase_ the resources requested by any job using these options. For that you will need your own configuration file. See [the nf-core website](https://nf-co.re/usage/configuration) for details.",
"properties": {
"max_cpus": {
"type": "integer",
"description": "Maximum number of CPUs that can be requested for any single job.",
"default": 16,
"fa_icon": "fas fa-microchip",
"hidden": true,
"help_text": "Use to set an upper-limit for the CPU requirement for each process. Should be an integer e.g. `--max_cpus 1`"
},
"max_memory": {
"type": "string",
"description": "Maximum amount of memory that can be requested for any single job.",
"default": "128.GB",
"fa_icon": "fas fa-memory",
"pattern": "^\\d+(\\.\\d+)?\\.?\\s*(K|M|G|T)?B$",
"hidden": true,
"help_text": "Use to set an upper-limit for the memory requirement for each process. Should be a string in the format integer-unit e.g. `--max_memory '8.GB'`"
},
"max_time": {
"type": "string",
"description": "Maximum amount of time that can be requested for any single job.",
"default": "240.h",
"fa_icon": "far fa-clock",
"pattern": "^(\\d+\\.?\\s*(s|m|h|d|day)\\s*)+$",
"hidden": true,
"help_text": "Use to set an upper-limit for the time requirement for each process. Should be a string in the format integer-unit e.g. `--max_time '2.h'`"
}
}
},
"generic_options": {
"title": "Generic options",
"type": "object",
Expand Down Expand Up @@ -278,9 +243,6 @@
{% if nf_core_configs %}{
"$ref": "#/$defs/institutional_config_options"
},{% endif %}
{
"$ref": "#/$defs/max_job_request_options"
},
{
"$ref": "#/$defs/generic_options"
}
Expand Down
14 changes: 13 additions & 1 deletion nf_core/pipelines/lint/nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ def nextflow_config(self) -> Dict[str, List[str]]:
* ``params.nf_required_version``: The old method for specifying the minimum Nextflow version. Replaced by ``manifest.nextflowVersion``
* ``params.container``: The old method for specifying the dockerhub container address. Replaced by ``process.container``
* ``igenomesIgnore``: Changed to ``igenomes_ignore``
* ``params.max_cpus``: Old method of specifying the maximum number of CPUs a process can request. Replaced by native Nextflow `resourceLimits`directive in config files.
* ``params.max_memory``: Old method of specifying the maximum number of memory can request. Replaced by native Nextflow `resourceLimits`directive.
* ``params.max_time``: Old method of specifying the maximum number of CPUs can request. Replaced by native Nextflow `resourceLimits`directive.

.. tip:: The ``snake_case`` convention should now be used when defining pipeline parameters

Expand Down Expand Up @@ -146,7 +149,13 @@ def nextflow_config(self) -> Dict[str, List[str]]:
["params.input"],
]
# Throw a warning if these are missing
config_warn = [["manifest.mainScript"], ["timeline.file"], ["trace.file"], ["report.file"], ["dag.file"]]
config_warn = [
["manifest.mainScript"],
["timeline.file"],
["trace.file"],
["report.file"],
["dag.file"],
]
# Old depreciated vars - fail if present
config_fail_ifdefined = [
"params.nf_required_version",
Expand All @@ -155,6 +164,9 @@ def nextflow_config(self) -> Dict[str, List[str]]:
"params.igenomesIgnore",
"params.name",
"params.enable_conda",
"params.max_cpus",
"params.max_memory",
"params.max_time",
]

# Lint for plugins
Expand Down
86 changes: 6 additions & 80 deletions tests/pipelines/lint/test_nextflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@
import re
from pathlib import Path

import yaml

import nf_core.pipelines.create.create
import nf_core.pipelines.lint
from nf_core.utils import NFCoreYamlConfig

from ..test_lint import TestLint

Expand All @@ -30,7 +27,6 @@ def test_default_values_match(self):
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0
assert len(result["warned"]) == 0
assert "Config default value correct: params.max_cpus" in str(result["passed"])
assert "Config default value correct: params.validate_params" in str(result["passed"])

def test_nextflow_config_bad_name_fail(self):
Expand Down Expand Up @@ -69,88 +65,16 @@ def test_nextflow_config_missing_test_profile_failed(self):
assert len(result["failed"]) > 0
assert len(result["warned"]) == 0

def test_default_values_fail(self):
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
"""Test linting fails if the default values in nextflow.config do not match the ones defined in the nextflow_schema.json."""
# Change the default value of max_cpus in nextflow.config
nf_conf_file = Path(self.new_pipeline) / "nextflow.config"
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(r"\bmax_cpus\s*=\s*16\b", "max_cpus = 0", content)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
# Change the default value of max_memory in nextflow_schema.json
nf_schema_file = Path(self.new_pipeline) / "nextflow_schema.json"
with open(nf_schema_file) as f:
content = f.read()
fail_content = re.sub(r'"default": "128.GB"', '"default": "18.GB"', content)
with open(nf_schema_file, "w") as f:
f.write(fail_content)
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 2
assert (
"Config default value incorrect: `params.max_cpus` is set as `16` in `nextflow_schema.json` but is `0` in `nextflow.config`."
in result["failed"]
)
assert (
"Config default value incorrect: `params.max_memory` is set as `18.GB` in `nextflow_schema.json` but is `128.GB` in `nextflow.config`."
in result["failed"]
)

def test_catch_params_assignment_in_main_nf(self):
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
"""Test linting fails if main.nf contains an assignment to a parameter from nextflow_schema.json."""
# Add parameter assignment in main.nf
main_nf_file = Path(self.new_pipeline) / "main.nf"
with open(main_nf_file, "a") as f:
f.write("params.max_time = 42")
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 1
assert (
result["failed"][0]
== "Config default value incorrect: `params.max_time` is set as `240.h` in `nextflow_schema.json` but is `null` in `nextflow.config`."
)

def test_allow_params_reference_in_main_nf(self):
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
"""Test linting allows for references like `params.aligner == 'bwa'` in main.nf. The test will detect if the bug mentioned in GitHub-issue #2833 reemerges."""
# Add parameter reference in main.nf
main_nf_file = Path(self.new_pipeline) / "main.nf"
with open(main_nf_file, "a") as f:
f.write("params.max_time == 42")
lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0

def test_default_values_ignored(self):
mirpedrol marked this conversation as resolved.
Show resolved Hide resolved
"""Test ignoring linting of default values."""
# Add max_cpus to the ignore list
nf_core_yml_path = Path(self.new_pipeline) / ".nf-core.yml"
nf_core_yml = NFCoreYamlConfig(
repository_type="pipeline", lint={"nextflow_config": [{"config_defaults": ["params.max_cpus"]}]}
)
with open(nf_core_yml_path, "w") as f:
yaml.dump(nf_core_yml.model_dump(), f)

lint_obj = nf_core.pipelines.lint.PipelineLint(self.new_pipeline)
lint_obj.load_pipeline_config()
lint_obj._load_lint_config()
result = lint_obj.nextflow_config()
assert len(result["failed"]) == 0
assert len(result["ignored"]) == 1
assert "Config default value correct: params.max_cpu" not in str(result["passed"])
assert "Config default ignored: params.max_cpus" in str(result["ignored"])

def test_default_values_float(self):
"""Test comparing two float values."""
# Add a float value `dummy=0.0001` to the nextflow.config below `validate_params`
nf_conf_file = Path(self.new_pipeline) / "nextflow.config"
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(
r"validate_params\s*=\s*true", "params.validate_params = true\ndummy = 0.000000001", content
r"validate_params\s*=\s*true",
"params.validate_params = true\ndummy = 0.000000001",
content,
)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
Expand Down Expand Up @@ -180,7 +104,9 @@ def test_default_values_float_fail(self):
with open(nf_conf_file) as f:
content = f.read()
fail_content = re.sub(
r"validate_params\s*=\s*true", "params.validate_params = true\ndummy = 0.000000001", content
r"validate_params\s*=\s*true",
"params.validate_params = true\ndummy = 0.000000001",
content,
)
with open(nf_conf_file, "w") as f:
f.write(fail_content)
Expand Down
Loading
Loading