riga / law

Build large-scale task workflows: luigi + job submission + remote targets + environment sandboxing using Docker/Singularity
http://law.readthedocs.io
BSD 3-Clause "New" or "Revised" License
98 stars 41 forks source link

Fix `threads` in HTCondor workflows #108

Closed HerrHorizontal closed 3 years ago

HerrHorizontal commented 3 years ago

Dirty fix for kwargs['threads'] in the job manager creation for HTCondor workflows. There certainly is a more elegant solution possible. This is only a dirty fix.

riga commented 3 years ago

Inferring from the changed line, I assume that you want the htcondor job manager to use only a single thread (which is the default of the HTCondorJobManager init). The line you removed prevents that the threads parameter value of the task is forwarded to that init method. However, the specific methods (e.g. BaseRemoteWorkflowProxy._submit) will still pass this value upstream since it might be subject to changes (e.g. self.job_manager.submit_batch in the BaseRemoteWorkflowProxy).

You can either set --threads 1 dynamically per task, or, to entirely fix this value to 1, set threads = 1 on the particular task or your personal HTCondorWorkflow base task.

Does that solve your problem?

HerrHorizontal commented 3 years ago

So wouldn't it be the best solution to set a default value threads=1 in the HTCondorJobManager? When I don't remove this from the kwargs, law complains that it doesn't exist.

riga commented 3 years ago

It's actually already there:

https://github.com/riga/law/blob/8469554c73124aea1f246ebc496ef8f80c60651e/law/contrib/htcondor/job.py#L38

I'm not really sure what the problem is. Can you post a stack trace or a small description of what you want to achieve?