Review SLURM configuration and include new configuration option for pre-sbatch commands and `gpus` #1678

tcompa · 2024-07-23T14:41:57Z

Checklist before merging

I added an appropriate entry to CHANGELOG.md
I merged main into the current branch.

github-actions · 2024-07-23T14:46:27Z

Benchmark comparison

GET /api/alive/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`
vanilla	1.0	0.8	1.25
power	1.0	0.8	1.25
dataset	1.0	0.8	1.25
project	1.0	0.8	1.25
job	0.8	0.8	1.00

GET /api/v2/dataset/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`	Size current (Kb)	Size `main` (Kb)
vanilla	8.6	8.4	1.02	0.7	0.7
power	412.5	413.2	1.00	440.2	440.2
dataset	161.7	161.3	1.00	171.2	171.2
project	47.4	46.7	1.01	58.9	58.9
job	27.3	30.2	0.90	41.7	41.7

GET /api/v2/job/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`	Size current (Kb)	Size `main` (Kb)
vanilla	5.6	5.6	1.00	2.2	2.2
power	136.3	139.8	0.97	652.0	652.0
dataset	139.4	141.9	0.98	904.8	904.8
project	46.7	47.3	0.99	283.7	283.7
job	36.1	37.2	0.97	224.8	224.8

GET /api/v2/project/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`	Size current (Kb)	Size `main` (Kb)
vanilla	6.0	6.0	1.00	0.1	0.1
power	5.9	5.8	1.02	0.1	0.1
dataset	5.9	5.8	1.02	0.1	0.1
project	7.8	7.8	1.00	2.3	2.3
job	5.8	5.9	0.98	0.1	0.1

GET /api/v2/workflow/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`	Size current (Kb)	Size `main` (Kb)
vanilla	10.0	10.5	0.95	0.8	0.8
power	23.5	24.4	0.96	29.8	29.8
dataset	18.2	18.4	0.99	17.1	17.1
project	23.9	23.6	1.01	21.5	21.5
job	9.8	9.8	1.00	0.9	0.9

POST /api/v2/project/3/dataset/405/images/query/

User	Time current (ms)	Time `main` (ms)	Ratio current/`main`	Size current (Kb)	Size `main` (Kb)
dataset	149.7	149.8	1.00	275.1	275.1

github-actions · 2024-07-23T14:54:40Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
fractal_server/app/runner/executors/slurm
_slurm_config.py	$This PR adds 4 to the number of statements in fractal_server/app/runner/executors/slurm/_slurm_config.py, taking it from 153 to 157.$	$This PR doesn't change the number of statements missing coverage in fractal_server/app/runner/executors/slurm/_slurm_config.py, which is 8.$	$This PR adds 0.13 percentage points to the coverage rate in fractal_server/app/runner/executors/slurm/_slurm_config.py, taking it from 94.77% (145/153) to 94.9% (149/157).$	$In this PR, 5 new statements are added to fractal_server/app/runner/executors/slurm/_slurm_config.py, 5 of which are covered (100%).$
fractal_server/app/runner/executors/slurm/ssh
executor.py	$This PR adds 10 to the number of statements in fractal_server/app/runner/executors/slurm/ssh/executor.py, taking it from 556 to 566.$	$This PR doesn't change the number of statements missing coverage in fractal_server/app/runner/executors/slurm/ssh/executor.py, which is 133.$	$This PR adds 0.42 percentage points to the coverage rate in fractal_server/app/runner/executors/slurm/ssh/executor.py, taking it from 76.07% (423/556) to 76.5% (433/566).$	$In this PR, 11 new statements are added to fractal_server/app/runner/executors/slurm/ssh/executor.py, 11 of which are covered (100%).$
fractal_server/app/runner/executors/slurm/sudo
executor.py	$This PR adds 2 to the number of statements in fractal_server/app/runner/executors/slurm/sudo/executor.py, taking it from 441 to 443.$	$This PR adds 6 to the number of statements missing coverage in fractal_server/app/runner/executors/slurm/sudo/executor.py, taking it from 35 to 41.$	$This PR removes 1.32 percentage points from the coverage rate in fractal_server/app/runner/executors/slurm/sudo/executor.py, taking it from 92.06% (406/441) to 90.74% (402/443).$	$In this PR, 2 new statements are added to fractal_server/app/runner/executors/slurm/sudo/executor.py, 1 of which are covered (50%).$	1126
fractal_server/app/runner/v2
__init__.py	$This PR doesn't change the number of statements in fractal_server/app/runner/v2/__init__.py, which is 211.$	$This PR doesn't change the number of statements missing coverage in fractal_server/app/runner/v2/__init__.py, which is 4.$	$This PR doesn't change the coverage rate in fractal_server/app/runner/v2/__init__.py, which is 98.1% (207/211).$	$In this PR, 1 new statements are added to fractal_server/app/runner/v2/__init__.py, 1 of which are covered (100%).$
fractal_server/app/runner/v2/_slurm_common
get_slurm_config.py	$This PR adds 72 statements to fractal_server/app/runner/v2/_slurm_common/get_slurm_config.py. The file did not seem to exist on the base branch.$	$This PR adds 2 statements missing coverage to fractal_server/app/runner/v2/_slurm_common/get_slurm_config.py. The file did not seem to exist on the base branch.$	$The coverage rate of fractal_server/app/runner/v2/_slurm_common/get_slurm_config.py is 97.22% (70/72). The file did not seem to exist on the base branch.$	$In this PR, 3 new statements are added to fractal_server/app/runner/v2/_slurm_common/get_slurm_config.py, 3 of which are covered (100%).$
fractal_server/app/runner/v2/_slurm_ssh
_submit_setup.py	$This PR doesn't change the number of statements in fractal_server/app/runner/v2/_slurm_ssh/_submit_setup.py, which is 10.$	$This PR doesn't change the number of statements missing coverage in fractal_server/app/runner/v2/_slurm_ssh/_submit_setup.py, which is 0.$	$This PR doesn't change the coverage rate in fractal_server/app/runner/v2/_slurm_ssh/_submit_setup.py, which is 100% (10/10).$	$In this PR, 1 new statements are added to fractal_server/app/runner/v2/_slurm_ssh/_submit_setup.py, 1 of which are covered (100%).$
fractal_server/app/runner/v2/_slurm_sudo
_submit_setup.py	$This PR adds 10 statements to fractal_server/app/runner/v2/_slurm_sudo/_submit_setup.py. The file did not seem to exist on the base branch.$	$This PR adds 0 statements missing coverage to fractal_server/app/runner/v2/_slurm_sudo/_submit_setup.py. The file did not seem to exist on the base branch.$	$The coverage rate of fractal_server/app/runner/v2/_slurm_sudo/_submit_setup.py is 100% (10/10). The file did not seem to exist on the base branch.$	$In this PR, 1 new statements are added to fractal_server/app/runner/v2/_slurm_sudo/_submit_setup.py, 1 of which are covered (100%).$
fractal_server/ssh
_fabric.py	$This PR adds 7 to the number of statements in fractal_server/ssh/_fabric.py, taking it from 150 to 157.$	$This PR doesn't change the number of statements missing coverage in fractal_server/ssh/_fabric.py, which is 0.$	$This PR doesn't change the coverage rate in fractal_server/ssh/_fabric.py, which is 100% (157/157).$	$In this PR, 7 new statements are added to fractal_server/ssh/_fabric.py, 7 of which are covered (100%).$
Project Total

_{This report was generated by python-coverage-comment-action}

tests/v2/00_ssh/test_FractalSSH.py

tcompa · 2024-07-24T06:48:49Z

Add log to non-SSH executor, regarding pre-submission commands
Fix current use of pre-submission commands for non-GPU tasks
Acquire lock for SFTP writing of new submission script
Test new gpus option
Verify persistence (or not) of module load between different calls

…xecutor

tcompa added 9 commits July 23, 2024 15:05

Fix f-string in log

5b2d25b

Improve check of gpu_slurm_config=None

8d8d9bf

Merge ssh/non-ssh redundant copies of get_slurm_config.py

1d59771

Rename v1 test

57a2977

Move get_slurm_config module to v2 scope

d0466d4

Remove obsolete directory arguments from get_slurm_config in v2

779e2d3

Fix typo in tests (paralell->parallel)

c3672dd

Remove obsolete argument from get_slurm_config call

1774d88

Add test_slurm_config for v2

c4cecb5

tcompa linked an issue Jul 23, 2024 that may be closed by this pull request

Expose task-specific pre-sbatch commands #1565

Closed

tcompa added 3 commits July 23, 2024 16:43

Rename app.runner.v2._slurm into app.runner.v2._slurm_sudo

060680e

Rename app.runner.v2._slurm into app.runner.v2._slurm_sudo

08ec232

Add init file for _slurm_common subpackage

8879240

tcompa added 5 commits July 23, 2024 17:13

Simplify conditional check in slurm config

baafe51

Include pre_submission_commands in slurm config models

3bf72b1

Include logic for pre-submission commands in ssh executor (ref #1565)

b819297

Add test_slurm_ssh_executor_submit_with_pre_sbatch

cbbbd70

Add test_run_command_session_persistence

5d0e6c3

tcompa commented Jul 23, 2024

View reviewed changes

tests/v2/00_ssh/test_FractalSSH.py Outdated Show resolved Hide resolved

tcompa added 2 commits July 24, 2024 08:08

Use pre_submission_commands attribute name consistently

9da7006

Include SLURM gpus parameter (ref #1679)

69998d5

tcompa added 6 commits July 24, 2024 08:57

Fix behvior for needs_gpu=False in slurm-config

00da10b

Add test_get_slurm_config_gpu_options

7621d83

Print warning when using pre_submission_commands in non-SSH SLURM e…

320f1a9

…xecutor

Introduce FractalSSH.write_remote_file

a3d698f

Introduce test for FractalSSH.write_remote_file

c29d5e6

Rename variable in fixture

4999de3

Simplify error message

e9f0e3f

tcompa changed the title ~~Review SLURM configuration and include new configuration option for pre-sbatch commands~~ Review SLURM configuration and include new configuration option for pre-sbatch commands and gpus Jul 24, 2024

tcompa mentioned this pull request Jul 24, 2024

Expose task-specific pre-sbatch commands #1565

Closed

tcompa added 2 commits July 24, 2024 10:38

Fix comment

37ebade

Update CHANGELOG [skip ci]

168ccb8

tcompa requested a review from mfranzon July 24, 2024 08:42

tcompa marked this pull request as ready for review July 24, 2024 08:42

tcompa added 4 commits July 24, 2024 10:44

Fix comment [skip ci]

6a117c1

Update CHANGELOG [skip ci]

bda4cd0

Drop obsolete test

fc467b9

Cover additional branch in write_remote_file method test

de4b037

tcompa merged commit 5407871 into main Jul 24, 2024
20 checks passed

tcompa deleted the 1565-expose-task-specific-pre-sbatch-commands branch July 24, 2024 09:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review SLURM configuration and include new configuration option for pre-sbatch commands and `gpus` #1678

Review SLURM configuration and include new configuration option for pre-sbatch commands and `gpus` #1678

tcompa commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

tcompa commented Jul 24, 2024 •

edited

Loading

Review SLURM configuration and include new configuration option for pre-sbatch commands and gpus #1678

Review SLURM configuration and include new configuration option for pre-sbatch commands and gpus #1678

Conversation

tcompa commented Jul 23, 2024 • edited Loading

Checklist before merging

github-actions bot commented Jul 23, 2024 • edited Loading

Benchmark comparison

GET /api/alive/

GET /api/v2/dataset/

GET /api/v2/job/

GET /api/v2/project/

GET /api/v2/workflow/

POST /api/v2/project/3/dataset/405/images/query/

github-actions bot commented Jul 23, 2024 • edited Loading

Coverage report

tcompa commented Jul 24, 2024 • edited Loading

Review SLURM configuration and include new configuration option for pre-sbatch commands and `gpus` #1678

Review SLURM configuration and include new configuration option for pre-sbatch commands and `gpus` #1678

tcompa commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

tcompa commented Jul 24, 2024 •

edited

Loading