Feat append arglist to srun #8

fordmcdonald · 2023-03-07T18:32:40Z

No description provided.

…ith default config

fordmcdonald · 2023-03-07T20:00:23Z

I worked under multiple branches for the attached issues, but I forgot to create a pull request for the default config commit. This PR covers the following features:

Added a default configuration file. The user can provide a custom config via --config <config.toml>, which will be merged with the default config for any missing attributes.
Appends an arglist of strings that reflects the parameters provided in the config. The function compileArgumentList(dict) handles the various formats and positional args that xnat2bids requires.
The default config has been updated with an exhaustive list of xnat2bids parameters. I have made a separate issue to build out handling all possible slurm arguments.

mirestrepo

Looks great! I added few comments ut this is pretty close!

run_xnat2bids.py

x2b_default_config.toml

mirestrepo · 2023-03-07T21:08:26Z

x2b_default_config.toml

+
+[xnat2bids-args]
+session = "XNAT_E00114"
+bids_root = "/gpfs/data/bnc/shared/bids-export/"


I think the defaults should match the defaults in our docs

is /gpfs/data/bnc/shared/bids-export/ our default? Maybe we should also make it in the $USER home so we don't have to manage cleaning up of bcc's folder

mirestrepo

So close!

run_xnat2bids.py

tests/test_xnat2bids.py

mirestrepo · 2023-03-08T15:58:29Z

tests/x2b_test_config.toml

@@ -0,0 +1,22 @@
+[slurm-args]


commit the values that we have good defaults for

x2b_example_user_config.toml

run_xnat2bids.py

mirestrepo

I'm sorry I keep suggesting stuff. Getting there! Love the changes you made to the compilation of args, volume bindings and logging

x2b_default_config.toml

mirestrepo · 2023-03-17T13:12:16Z

x2b_default_config.toml

+
+[xnat2bids-args]
+session = "XNAT_E00114"
+bids_root = "/gpfs/data/bnc/shared/bids-export/"


is /gpfs/data/bnc/shared/bids-export/ our default? Maybe we should also make it in the $USER home so we don't have to manage cleaning up of bcc's folder

x2b_default_config.toml

x2b_example_user_config.toml

run_xnat2bids.py

mirestrepo

Even closer. Tiny changes left. I'm changing my mind here, but maybe the default config should have good defaults for any example, in that case, the session is probably best not part of the default

mirestrepo · 2023-03-30T18:59:01Z

tests/x2b_test_config.toml

+bids_root_dir=""
+host="https://xnat.bnc.brown.edu"
+session-suffix=-1
+bidsmap-file="/gpfs/data/bnc/shared/scripts/fmcdona4/bidsmap.json"


I don't see this file in Oscar. That location has a txt but it's not the bidsmap format. Also I don't think we are using a bidsmap file for demodat

Here, bidsmap-file is just dummy data, testing that bidsmap filepaths are being correctly adding to bindings. In test_xnat2bids.py, I'm only asserting that the filepath has been added to bindings after making a call to compileArgumentList.

mirestrepo · 2023-03-30T19:01:58Z

tests/x2b_test_config.toml

+nodes = 1
+cpus-per-task = 2
+job-name = "xnat2bids"
+output = "/gpfs/data/bnc/scratch/logs/%J.txt"


Suggested change

output = "/gpfs/data/bnc/scratch/logs/%J.txt"

output = "/gpfs/scratch/%u/logs/xnat2bids_test%J.log"

run_xnat2bids.py

tests/x2b_test_config.toml

mirestrepo · 2023-03-30T19:11:07Z

x2b_default_config.toml

+mem = 16000
+nodes = 1
+cpus-per-task = 2
+job-name = "xnat2bids"


Suggested change

job-name = "xnat2bids"

job-name = "xnat2bids"

output = "/gpfs/scratch/%u/logs/xnat2bids%J.log"

In PR #11, I'm not specifying an output parameter in slurm-args, so that I can add the session # to the logging statements.

if not ('output' in arg_dict['slurm-args']): output = f"/users/{user}/logs/%x-{session}-%J.txt" arg = f"--output {output}" slurm_param_list.append(arg)

But I like the idea of using %u, and letting slurm define user here. Should logs go to scratch/user/logs vs. /users/user/logs ?

x2b_example_user_config.toml

x2b_default_config.toml

Co-authored-by: Maria Isabel Restrepo <[email protected]>

…essions

Feat batch multiple sessions

Ford McDonald added 2 commits March 6, 2023 14:36

feat(run_xnat2bids.py): add cli arg to parse user config and merger w…

8759e42

…ith default config

feat: append xnat2bids parameters to arglist

db0fb46

This was linked to issues Mar 7, 2023

Append xnat2bids parameter tags to arglist #6

Closed

Add default config for xnat2bids script #5

Open

fordmcdonald self-assigned this Mar 7, 2023

fordmcdonald added the enhancement New feature or request label Mar 7, 2023

fordmcdonald marked this pull request as ready for review March 7, 2023 20:00

fordmcdonald requested a review from mirestrepo March 7, 2023 20:00

mirestrepo requested changes Mar 7, 2023

View reviewed changes

Ford McDonald added 2 commits March 7, 2023 21:13

resolve change requests and add unit test

7180467

point default config to scripts directory

f94ed6c

mirestrepo requested changes Mar 8, 2023

View reviewed changes

mirestrepo reviewed Mar 8, 2023

View reviewed changes

run_xnat2bids.py Outdated Show resolved Hide resolved

Ford McDonald added 2 commits March 8, 2023 16:41

updates singularity path binding and adds logging

3c90cea

generic user mail for tests

1e4fe1c

fordmcdonald requested a review from mirestrepo March 8, 2023 22:03

mirestrepo requested changes Mar 17, 2023

View reviewed changes

fix: resolve change requests

a84255a

fordmcdonald requested a review from mirestrepo March 21, 2023 20:05

mirestrepo added this to the April BNC Workshop milestone Mar 22, 2023

Ford McDonald added 6 commits March 27, 2023 18:31

handle multiple sessions and verbose flag

c626f43

add asyncio to launch all jobs concurrently

9a0acc0

adding example config for processing multiple sessions

cfd1580

turn off asyncio debug logging

689c74f

srun in quiet mode, removed await for child process to complete

1f7858c

add session number to output log filenames

26d5f2f

mirestrepo reviewed Mar 30, 2023

View reviewed changes

Ford McDonald added 2 commits April 3, 2023 17:02

add structured logging enabled by xnat2bids verbosity and other tuning

cbd8d02

remove dead code

e0930ff

fordmcdonald and others added 11 commits April 3, 2023 17:30

Update run_xnat2bids.py

6b625c4

Co-authored-by: Maria Isabel Restrepo <[email protected]>

Update run_xnat2bids.py

8c0cde3

Co-authored-by: Maria Isabel Restrepo <[email protected]>

logging comments

7574530

Update x2b_default_config.toml

156a87f

Co-authored-by: Maria Isabel Restrepo <[email protected]>

Update x2b_example_user_config.toml

a18de78

Co-authored-by: Maria Isabel Restrepo <[email protected]>

Update x2b_default_config.toml

caca52f

Co-authored-by: Maria Isabel Restrepo <[email protected]>

update unit tests with bug fixes

9eb3548

define bids_root if provided in config

0a48161

handle user specified version

9589eab

Merge branch 'feat-append-arglist-to-srun' into feat-batch-multiple-s…

2f6b262

…essions

Merge pull request #11 from brown-bnc/feat-batch-multiple-sessions

e8dbccd

Feat batch multiple sessions

fordmcdonald closed this Apr 13, 2023

fordmcdonald deleted the feat-append-arglist-to-srun branch April 13, 2023 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat append arglist to srun #8

Feat append arglist to srun #8

fordmcdonald commented Mar 7, 2023

fordmcdonald commented Mar 7, 2023 •

edited

Loading

mirestrepo left a comment

mirestrepo Mar 7, 2023

mirestrepo Mar 17, 2023

mirestrepo left a comment

mirestrepo Mar 8, 2023

mirestrepo left a comment

mirestrepo Mar 17, 2023

mirestrepo left a comment

mirestrepo Mar 30, 2023

fordmcdonald Apr 3, 2023

mirestrepo Mar 30, 2023

mirestrepo Mar 30, 2023

fordmcdonald Apr 3, 2023 •

edited

Loading

	output = "/gpfs/data/bnc/scratch/logs/%J.txt"
	output = "/gpfs/scratch/%u/logs/xnat2bids_test%J.log"

	job-name = "xnat2bids"
	job-name = "xnat2bids"
	output = "/gpfs/scratch/%u/logs/xnat2bids%J.log"

Feat append arglist to srun #8

Feat append arglist to srun #8

Conversation

fordmcdonald commented Mar 7, 2023

fordmcdonald commented Mar 7, 2023 • edited Loading

mirestrepo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirestrepo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirestrepo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mirestrepo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fordmcdonald Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

fordmcdonald commented Mar 7, 2023 •

edited

Loading

fordmcdonald Apr 3, 2023 •

edited

Loading