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

Use job scheduling information to allow more flexible job submission #77

Merged
merged 49 commits into from
Jan 31, 2024

Conversation

thomasmfish
Copy link
Collaborator

@thomasmfish thomasmfish commented Jan 15, 2024

This is to enable me to use the JobScheduler for B24. The main aim of this was for me to be able to manually specify and and run a bunch of jobs in the same scheduler in a way that doesn't rely on slicing (e.g. I am likely to want 3 jobs for different IMOD reconstructions and 1 job for AreTomo reconstructions to be run all in one).

This has some changes that will affect existing usage, so I wanted to start a discussion before going further with the JobController. The basics of this are:

  • JobSchedulingInformation class that contains all the job information required for submission, as well as the job status information.
  • SchedulerModeInterface is now JobSlicerInterface and the function just accepts slicer_params and an input JobSchedulingInformation and outputs a list of JobSchedulingInformations.
  • ProgramWrapper no longer has the set_cores and get_args methods as that is handled by either setting them in the initial JobSchedulingInformation object, or when create_sliced_processing_jobs or create_sliced_aggregating_jobs are called (these output lists of JobSchedulingInformation objects).

To do:

  • JobController needs some work, along with run_ppc, and tests for those. This is where I need to know what's easiest for existing projects - perhaps there's some compatibility layer I can add to avoid updating the wrappers?
  • I want to make sure all the typing is done in a more standardised way (there's a mix of styles e.g. list[int] and List[int]).

Questions:

  • Does all of this sound okay and usable?
  • Are there any changes that would make this fit better with current usage?
  • What is the best way of moving ahead with JobController? There have been some changes there but the handling of the resource setting via the wrappers has been a point where I'm unsure of how to handle it, especially since I wont be using that part.
  • Is this best not being merged but being a fork or something?

@thomasmfish
Copy link
Collaborator Author

All the tests are passing for me too

@PeterC-DLS
Copy link
Collaborator

@VictoriaBeilsten-Edmands , could you review this too to update yourself with the changes?

Copy link
Collaborator

@VictoriaBeilsten-Edmands VictoriaBeilsten-Edmands left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good and makes sense to me.

@thomasmfish
Copy link
Collaborator Author

@PeterC-DLS Please merge this whenever you're ready (I'd ask you to approve but why then wait for me to click a button?)

@PeterC-DLS
Copy link
Collaborator

I was going to test with a real submission job but got side-tracked by type checking...

@thomasmfish
Copy link
Collaborator Author

Okay, no worries, I just didn't want you to be waiting on me!

@PeterC-DLS PeterC-DLS force-pushed the Use-`JobSchedulingInformation` branch from 7a1af60 to a733fde Compare January 28, 2024 14:36
Add back script names for some job slicers
@PeterC-DLS PeterC-DLS force-pushed the Use-`JobSchedulingInformation` branch 2 times, most recently from cd242ac to 4e17462 Compare January 28, 2024 15:51
@PeterC-DLS PeterC-DLS force-pushed the Use-`JobSchedulingInformation` branch from 4e17462 to 52e1b59 Compare January 28, 2024 16:10
Fix wrong destination of output in pass thru wrapper, and more logging info
@PeterC-DLS PeterC-DLS force-pushed the Use-`JobSchedulingInformation` branch from c3682a3 to 05d8459 Compare January 29, 2024 09:11
Add documentation on testing and rename test env var for slurm partition
@PeterC-DLS PeterC-DLS force-pushed the Use-`JobSchedulingInformation` branch from 05d8459 to e7ee00e Compare January 29, 2024 09:54
thomasmfish

This comment was marked as resolved.

PeterC-DLS and others added 2 commits January 29, 2024 14:24
Simplify output path in pass thru wrapper, remove odd bits
@thomasmfish
Copy link
Collaborator Author

thomasmfish commented Jan 29, 2024

Just pushed a small change where a couple of error messages were missing the f to make it an f-string but otherwise I'm happy with everything

EDIT: Just saw the but about the n/a path, so pending that

@thomasmfish thomasmfish merged commit 12c62e8 into main Jan 31, 2024
1 check passed
@PeterC-DLS PeterC-DLS deleted the Use-`JobSchedulingInformation` branch January 31, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants