Correctly handle float in (Slurm|PBS)IO._convert_time_to_str

[janosh]

to set the time limit to half a day, i was using

resources = QResources(
    # ...
    time_limit=60 * 60 * 24 * 0.5,  # in seconds
)

and encountered

│     remote = {                                                                                                  │
│                  'step_attempts': 2,                                                                            │
│                  'retry_time_limit': '2024-07-24 19:13',                                                        │
│                  'error': Traceback (most recent call last):                                                    │
│                File                                                                                             │
│              "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/jobs/jobcontroller.py",   │
│              line 3340, in lock_job_for_update                                                                  │
│                  yield lock                                                                                     │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/jobs/runner.py",   │
│              line 539, in advance_state                                                                         │
│                  states_methods[state](lock)                                                                    │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/jobs/runner.py",   │
│              line 686, in submit                                                                                │
│                  submit_result = queue_manager.submit(                                                          │
│                                  ^^^^^^^^^^^^^^^^^^^^^                                                          │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/remote/queue.py",  │
│              line 153, in submit                                                                                │
│                  script_fpath = self.write_submission_script(                                                   │
│                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                   │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/remote/queue.py",  │
│              line 184, in write_submission_script                                                               │
│                  script_str = self.get_submission_script(                                                       │
│                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                       │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/jobflow_remote/remote/queue.py",  │
│              line 99, in get_submission_script                                                                  │
│                  return self.scheduler_io.get_submission_script(commands_list, options)                         │
│                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                         │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/qtoolkit/io/base.py", line 57, in │
│              get_submission_script                                                                              │
│                  if header := self.generate_header(options):                                                    │
│                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                     │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/qtoolkit/io/base.py", line 79, in │
│              generate_header                                                                                    │
│                  options = self.check_convert_qresources(options)                                               │
│                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                               │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/qtoolkit/io/base.py", line 186,   │
│              in check_convert_qresources                                                                        │
│                  return self._convert_qresources(resources)                                                     │
│                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                     │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/qtoolkit/io/slurm.py", line 582,  │
│              in _convert_qresources                                                                             │
│                  header_dict["time"] = self._convert_time_to_str(resources.time_limit)                          │
│                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                          │
│                File "/scratch/janosh/.venv/py312/lib/python3.12/site-packages/qtoolkit/io/slurm.py", line 546,  │
│              in _convert_time_to_str                                                                            │
│                  days = time.days                                                                               │
│                         ^^^^^^^^^                                                                               │
│              AttributeError: 'float' object has no attribute 'days'                                             │
│                                                                                                                 │
│              }                                                                                                  │
│    run_dir = '/scratch/janosh/jobs/35/ff/e2/35ffe263-f048-45e9-8479-a8f97fa69435_1'  

this PR changes

- if isinstance(time, int):
+ if not isinstance(time, timedelta):
    time = timedelta(seconds=time)

another option worth considering would be if isinstance(time, float):

[gpetretto]

Thanks!
It may be good to also change the type accepted in the method from int | timedelta to int | float | timedelta?

[janosh]

yes, meant to do that but forgot

[davidwaroquiers]

Thx @janosh
Just very small thing, if you pass a float, it will be rounded down (e.g. I want a walltime of 2.9 seconds, you'll get 2 seconds). I guess slurm/pbs don't allow fractional seconds anyway and it's up to the second ... So I guess we don't want/need to do something to round it up instead.
Just out of curiosity (because you added a test), what does a negative value mean here ? is there a use case in slurm or pbs for it ?

[janosh]

rounding is probably more hassle than it's worth

Just out of curiosity (because you added a test), what does a negative value mean here ? is there a use case in slurm or pbs for it ?

just to catch if the functionality ever changes in the future. now we'll be alerted, even if it's not intended behavior. i'll add a comment to this effect

[davidwaroquiers]

All good to me! (agreed that rounding is not worth ;))

[davidwaroquiers]

Thx!

[gpetretto]

Thanks!