feat(pipeline): define general time limit attribute #212

pbelmann · 2022-07-21T11:30:42Z

This PR introduces a concept that addresses point 3 in #210 and solves #199 .
The current concept is only applied to the quality control module as a general demonstration.
If this PR is merged, this concept can and should be applied to all processes.

Example Configuration

steps:
  qc:
    fastp:
       # Example params: " --cut_front --cut_tail --detect_adapter_for_pe  "
       additionalParams: "  "
       timeLimit: "AUTO"

The user is able to the following values for the timeLimit attribute:

DISABLED: Disable any time limit
AUTO (default): If the default resource configuration of a process is reduced then the time limit is scaled up.
NUMBER: A numeric value (in hours) can be directly set by the user.

Before merge:

The PR must be reviewed by one of the team members.
Please check if anything in the Readme must be adjusted, or added (development-setup, production-setup, user-guide).
PRs with new modules or workflow interfaces must include tests according to the developer guidelines.
The new code is readable, well commented and should adhere to our developer guidelines.
Before merging it must be checked if a squash of commits is required.

bosterholz

Top! Can be merged after comments are resolved.

bosterholz · 2022-07-29T10:41:14Z

lib/Utils.groovy

+          def timeLimit = defaultCPUs/flavor.cpus * processDefaults.time;
+          return timeLimit + timeUnit;
+        } else {
+          return "";


I don't get this else part. If my chosen flavor has more cores than default, then no time limit is set?
Issue #199 could happen again with no time limit.
I would think that in this case the default time should be used.

bosterholz · 2022-07-29T10:46:26Z

docs/developer_guidelines.md

@@ -73,11 +73,12 @@ Given a version number MAJOR.MINOR.PATCH, increment the:

 ## Process

-1. Process names should start `p`.
+Process names should start `p`. The input and output of processes should contain a sample and/or bin and contig id.


Suggested change

Process names should start `p`. The input and output of processes should contain a sample and/or bin and contig id.

Process names should start with a `p`. The in- and output of processes should contain a sample and/or a bin and contig id.

bosterholz · 2022-07-29T10:47:30Z

docs/developer_guidelines.md


-2. The input and output of processes should contain a sample and/or bin and contig id.
+### Pocesses should publish process specific files


Suggested change

### Pocesses should publish process specific files

### Processes should publish process specific files

bosterholz · 2022-07-29T10:47:51Z

docs/developer_guidelines.md


-3. Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.
+Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.


Suggested change

Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.

Processes should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.

bosterholz · 2022-07-29T10:50:05Z

docs/developer_guidelines.md

-4. Custom error strategies that do not follow the strategy defined in nextflow.config, should be documented (see Megahit example).
+### Time Limit
+
+Every process must define time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment  


Suggested change

Every process must define time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment

Every process must define a time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment

bosterholz · 2022-07-29T10:50:34Z

docs/developer_guidelines.md

+Every process must define time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment  
+which could lead to an endless execution of the process.
+
+You can use setTimeLimit helper method to add a user configurable time limit.


Suggested change

You can use setTimeLimit helper method to add a user configurable time limit.

You can use the setTimeLimit helper method to add a user configurable time limit.

pbelmann requested a review from bosterholz July 22, 2022 07:17

bosterholz approved these changes Jul 29, 2022

View reviewed changes

pbelmann force-pushed the feature/time-limit-handling branch from 18803a6 to ac800a8 Compare August 1, 2022 08:32

feat(pipeline): add general time limit handling

5fd0cb9

pbelmann force-pushed the feature/time-limit-handling branch from ac800a8 to 5fd0cb9 Compare August 1, 2022 08:56

pbelmann merged commit 509f1e3 into dev Aug 2, 2022

pbelmann deleted the feature/time-limit-handling branch August 2, 2022 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pipeline): define general time limit attribute #212

feat(pipeline): define general time limit attribute #212

pbelmann commented Jul 21, 2022

bosterholz left a comment

bosterholz Jul 29, 2022

pbelmann Jul 31, 2022

bosterholz Jul 29, 2022

bosterholz Jul 29, 2022

bosterholz Jul 29, 2022

bosterholz Jul 29, 2022

bosterholz Jul 29, 2022

	Process names should start `p`. The input and output of processes should contain a sample and/or bin and contig id.
	Process names should start with a `p`. The in- and output of processes should contain a sample and/or a bin and contig id.


		2. The input and output of processes should contain a sample and/or bin and contig id.
		### Pocesses should publish process specific files

	### Pocesses should publish process specific files
	### Processes should publish process specific files


		3. Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.
		Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.

	Pocesses should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.
	Processes should publish `.command.sh`, `.command.out`, `.command.log` and `.command.err` files but never `.command.run`.

	Every process must define time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment
	Every process must define a time limit which will never be reached on "normal" execution. This limit is only useful for errors in the execution environment

	You can use setTimeLimit helper method to add a user configurable time limit.
	You can use the setTimeLimit helper method to add a user configurable time limit.

feat(pipeline): define general time limit attribute #212

feat(pipeline): define general time limit attribute #212

Conversation

pbelmann commented Jul 21, 2022

bosterholz left a comment

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment

pbelmann Jul 31, 2022

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment

bosterholz Jul 29, 2022

Choose a reason for hiding this comment