search

mllg / batchtools

Tools for computation on batch systems
https://mllg.github.io/batchtools/
GNU Lesser General Public License v3.0
170 stars 51 forks source link

do not divide by 60 #218

Closed tdhock closed 5 years ago

tdhock commented 5 years ago

comments/docs say that walltime resource is in minutes. it should therefore be passed directly to --time parameter (without dividing by 60) because time expects minutes. see https://slurm.schedmd.com/sbatch.html

       -t, --time=<time>
              Set a limit on the total run time of the job allocation.  If the requested time  limit
              exceeds  the partition’s time limit, the job will be left in a PENDING state (possibly
              indefinitely).  The default time limit is the partition’s default  time  limit.   When
              the  time  limit  is  reached,  each task in each job step is sent SIGTERM followed by
              SIGKILL.  The interval between signals is specified by the Slurm configuration parame-
              ter  KillWait.   The  OverTimeLimit  configuration parameter may permit the job to run
              longer than scheduled.  Time resolution is one minute and second values are rounded up
              to the next minute.

              A  time limit of zero requests that no time limit be imposed.  Acceptable time formats
              include   "minutes",   "minutes:seconds",    "hours:minutes:seconds",    "days-hours",
              "days-hours:minutes" and "days-hours:minutes:seconds".

The important part is Acceptable time formats include "minutes", which means that raw numbers given to the --time parameter are interpreted as minutes

Also you probably want to fix the relevant part of the other slurm templates

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 87.506% when pulling aef3dd805119eb985acb0dd45bee2f97769070c1 on tdhock:patch-1 into da3988edd1589506562ae0c633d2a04f38cc5172 on mllg:master.

mllg commented 5 years ago

batchtools expects walltimes in seconds, so I believe the division by 60 is correct here. However, the comments in the template files are obviously wrong.

tdhock commented 5 years ago

ok well in that case I would suggest changing the comments in that template please

mllg commented 5 years ago

I just ran a test and can confirm that dividing by 60 is correct. I fixed the comments in the template. Thanks for reporting.