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

LSF scheduler seems to have redundant memory keyword #205

Open
josephhardinee opened this issue Nov 27, 2018 · 5 comments
Open

LSF scheduler seems to have redundant memory keyword #205

josephhardinee opened this issue Nov 27, 2018 · 5 comments
Labels

Comments

@josephhardinee
Copy link
Contributor

It looks like specifying the lsf scheduler requires two memory keywords (mem, and memory). Should mem on the LSF scheduler be turned into memory so that it lines up with other schedulers and the core object?

@josephhardinee
Copy link
Contributor Author

Also it appears that memory should be of form '24 GB', but mem need to be passed as an integer number of bytes.

@guillaumeeb
Copy link
Member

Historically, the memory keyword was for the dask-worker command --memory-limit option. Now it is the overall memory that should be used by all the dask worker processes. In default cases in most dask-jobqueue systems implementations, it is also used as the memory reservation for the scheduler. But in every implementations, you have either a resource_spec, job_mem, mem kwarg that can be used to override this value if the user wants to, but you can leave it as None.

We discussed leaving or not the duplicated ncpus or mem kwarg in #82 or more recently in #172 (comment), and maybe in #181 too.

I'm still undecided on this, especially for the memory kwarg. It seems unlikely that a user wants to have a different reservation that what it uses on dask_side. And in this case, maybe he could implement it with job_extra kwarg.

@josephhardinee
Copy link
Contributor Author

Okay, I don't have a strong preference either way except that I got a little confused.

Also I assume we would want the input form for the two to match (Using unit in the name as a string, vs passing an int for memory). To update this I assume we'd want to test the type for an int or str on mem keyword and do the conversion that way so as to not break previously existing scripts? Or would you rather it is just updated for both to take a str?

@guillaumeeb
Copy link
Member

Hm, I think it would be a minor breaking change and not impact a lot of people. I'd be more in favor of just update mem to be a string, and update the code using it.

@guillaumeeb
Copy link
Member

I think we can do two things to clarify this:

  • Update the documentation and examples to explain that for each job queuing system, there are workers kwargs, and job related kwargs.
  • For LSF, modify the ncpus and mem kwarg, prefixing them using job_.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants