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

Feat append arglist to srun #8

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8759e42
feat(run_xnat2bids.py): add cli arg to parse user config and merger w…
Mar 6, 2023
db0fb46
feat: append xnat2bids parameters to arglist
Mar 7, 2023
7180467
resolve change requests and add unit test
Mar 8, 2023
f94ed6c
point default config to scripts directory
Mar 8, 2023
3c90cea
updates singularity path binding and adds logging
Mar 8, 2023
1e4fe1c
generic user mail for tests
Mar 8, 2023
a84255a
fix: resolve change requests
Mar 21, 2023
c626f43
handle multiple sessions and verbose flag
Mar 27, 2023
9a0acc0
add asyncio to launch all jobs concurrently
Mar 28, 2023
cfd1580
adding example config for processing multiple sessions
Mar 28, 2023
689c74f
turn off asyncio debug logging
Mar 28, 2023
1f7858c
srun in quiet mode, removed await for child process to complete
Mar 29, 2023
26d5f2f
add session number to output log filenames
Mar 29, 2023
cbd8d02
add structured logging enabled by xnat2bids verbosity and other tuning
Apr 3, 2023
e0930ff
remove dead code
Apr 3, 2023
6b625c4
Update run_xnat2bids.py
fordmcdonald Apr 3, 2023
8c0cde3
Update run_xnat2bids.py
fordmcdonald Apr 3, 2023
7574530
logging comments
Apr 3, 2023
156a87f
Update x2b_default_config.toml
fordmcdonald Apr 3, 2023
a18de78
Update x2b_example_user_config.toml
fordmcdonald Apr 3, 2023
caca52f
Update x2b_default_config.toml
fordmcdonald Apr 3, 2023
9eb3548
update unit tests with bug fixes
Apr 6, 2023
0a48161
define bids_root if provided in config
Apr 6, 2023
9589eab
handle user specified version
Apr 6, 2023
2f6b262
Merge branch 'feat-append-arglist-to-srun' into feat-batch-multiple-s…
fordmcdonald Apr 11, 2023
e8dbccd
Merge pull request #11 from brown-bnc/feat-batch-multiple-sessions
fordmcdonald Apr 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
274 changes: 233 additions & 41 deletions run_xnat2bids.py
Original file line number Diff line number Diff line change
@@ -1,57 +1,249 @@
import argparse
import asyncio
import copy
from collections import defaultdict
from getpass import getpass
import glob
import logging
import os
import pathlib
import shlex
import shutil
import subprocess
from toml import load

# Load config file into dictionary
arglist = load('x2b_config.toml')

# Extract arglist from dictionary
for key in arglist:

# Populate sbatch arguments
if (key == "sbatch-args"):
time = arglist[key]['time']
mem = arglist[key]['mem']
nodes = arglist[key]['nodes']
cpus = arglist[key]['cpus']
job = arglist[key]['job']
output = arglist[key]['output']
email = arglist[key]['email']

# Populate xnat2bids arguments
if(key == "xnat2bids-args"):
version = arglist[key]['version']
data_dir = arglist[key]['data_dir']
bids_root = arglist[key]['bids_root']
session = arglist[key]['session']

# Fetch latest version if not provided
if(version == ""):
logging.basicConfig(format='%(levelname)s: %(message)s', level=logging.NOTSET)
logging.basicConfig(level=logging.INFO)
logging.getLogger('asyncio').setLevel(logging.WARNING)

def setLoggingLevel(x2b_arglist: list):
if "--verbose" in x2b_arglist:
logging.getLogger().setLevel(logging.DEBUG)
else:
logging.getLogger().setLevel(logging.INFO)

def fetchLatestVersion():
list_of_versions = glob.glob('/gpfs/data/bnc/simgs/brownbnc/*')
latest_version = max(list_of_versions, key=os.path.getctime)
version = latest_version.split('-')[-1].replace('.sif', '')
return (latest_version.split('-')[-1].replace('.sif', ''))

def extractParams(param, value):
arg = []
for v in value:
arg.append(f"--{param} {v}")
return ' '.join(arg)

def mergeConfigFiles(user_cfg, default_cfg):

user_slurm = user_cfg['slurm-args']
user_x2b = user_cfg['xnat2bids-args']
default_slurm = default_cfg['slurm-args']
default_x2b = default_cfg['xnat2bids-args']

# Assemble merged dictionary with default values.
merged_dict = defaultdict(dict)
merged_dict['xnat2bids-args'].update(default_x2b)
merged_dict['slurm-args'].update(default_slurm)

# Update merged dictionary with user provided arguments.
merged_dict['slurm-args'].update(user_slurm)
merged_dict['xnat2bids-args'].update(user_x2b)

# Add session specific parameter blocks
for key in user_cfg.keys():
if key == 'slurm-args' or key == 'xnat2bids-args':
continue
merged_dict[key].update(user_cfg[key])

return merged_dict

def compileX2BArgList(xnat2bids_dict, session, bindings):
x2b_param_list = []
param_lists = ["includeseq", "skipseq"]

for param, value in xnat2bids_dict.items():
if value == "" or value is None:
continue
# Set {session} as first parameter
elif param == "sessions":
x2b_param_list.insert(0,session)
# Set {bids_root} as second parameter
elif param == "bids_root":
arg = f"{value}"
bindings.append(arg)
x2b_param_list.insert(1, arg)
elif param == "bidsmap-file":
arg = f"--{param} {value}"
bindings.append(value)
x2b_param_list.append(arg)
# Set as many verbose flags as specified.
elif param == "verbose":
arg = f"--{param}"
for i in range(value):
x2b_param_list.append(arg)
# If overwrite is equal to true, set flag.
elif param == "overwrite":
arg = f"--{param}"
if value == True: x2b_param_list.append(arg)
# If version is specified, continue
elif param == "version":
continue
# Extract parameters from include / skip lists
elif param in param_lists:
arg = extractParams(param, value)
x2b_param_list.append(arg)
# Other arguments follow --param value format.
else:
arg = f"--{param} {value}"
x2b_param_list.append(arg)

return x2b_param_list

def compileArgumentList(session, arg_dict, user):
"""Create command line argument list from TOML dictionary."""

# Create copy of dictionary, so as not to update
# the original object reference while merging configs.
arg_dict_copy = copy.deepcopy(arg_dict)

slurm_param_list = []
bindings = []
# Compile list of appended arguments
x2b_param_dict = {}
for section_name, section_dict in arg_dict_copy.items():
# Extract xnat2bids-args from original dictionary
if section_name == "xnat2bids-args":
x2b_param_dict = section_dict

# Extract slurm-args from original dictionary
elif section_name == "slurm-args":
for param, value in section_dict.items():
if value != "" and value is not None:
arg = f"--{param} {value}"
slurm_param_list.append(arg)

# If a session key exist for the current session being
# processed, update final config with session block.
elif section_name == session:
x2b_param_dict.update(section_dict)

# Transform session config dictionary into argument list.
x2b_param_list = compileX2BArgList(x2b_param_dict, session, bindings)
return x2b_param_list, slurm_param_list, bindings

async def main():
# Instantiate argument parserß
parser = argparse.ArgumentParser()
parser.add_argument("--config", help="path to user config")
args = parser.parse_args()

# Load default config file into dictionary
script_dir = pathlib.Path(__file__).parent.resolve()
default_params = load(f'{script_dir}/x2b_default_config.toml')

# Set arg_dict. If user provides config, merge dictionaries.
if args.config is None:
arg_dict = default_params
else:
# Load user configuration
user_params = load(args.config)

# Merge with default configuration
arg_dict = mergeConfigFiles(user_params, default_params)

# If sessions does not exist in arg_dict, prompt user for Accession ID(s).
if 'sessions' not in arg_dict['xnat2bids-args']:
sessions_input = input("Enter Session(s) (comma separated): ")
arg_dict['xnat2bids-args']['sessions'] = [s.strip() for s in sessions_input.split(',')]


# Fetch user credentials
user = input('Enter XNAT Username: ')
password = getpass('Enter Password: ')

# Assemble parameter lists per session
argument_lists = []
for session in arg_dict['xnat2bids-args']['sessions']:

# Fetch compiled xnat2bids and slurm parameter lists
x2b_param_list, slurm_param_list, bindings = compileArgumentList(session, arg_dict, user)

# Insert username and password into x2b_param_list
x2b_param_list.insert(2, f"--user {user}")
x2b_param_list.insert(3, f"--pass {password}")

# Fetch latest version if not provided
if not ('version' in arg_dict['xnat2bids-args']):
version = fetchLatestVersion()
else:
version = arg_dict['xnat2bids-args']['version']

# Define singularity image
simg=f"/gpfs/data/bnc/simgs/brownbnc/xnat-tools-{version}.sif"

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

if not (os.path.exists(os.path.dirname(output))):
os.mkdir(os.path.dirname(output))

# Define bids root directory
if not ('bids_root' in arg_dict['xnat2bids-args']):
bids_root = f"/users/{user}/bids-export/"
x2b_param_list.insert(1, bids_root)
bindings.append(bids_root)
else:
bids_root = x2b_param_list[1]

if not (os.path.exists(os.path.dirname(bids_root))):
os.mkdir(os.path.dirname(bids_root))

# Store xnat2bids, slurm, and binding paramters as tuple.
argument_lists.append((x2b_param_list, slurm_param_list, bindings))

# Set logging level per session verbosity.
setLoggingLevel(x2b_param_list)

logging.debug({
"message": "Argument List",
"session": session,
"slurm_param_list": slurm_param_list,
"x2b_param_list": x2b_param_list,

})

# Loop over argument lists for provided sessions.
tasks = []
for args in argument_lists:
# Compile bindings into formated string
bindings_str = ' '.join(f"-B {path}" for path in args[2])

# Fetch user credentials
user = input('Enter Username: ')
password = getpass('Enter Password: ')
# Process command string for SRUN
srun_cmd = shlex.split(f"srun -Q {' '.join(args[1])} \
singularity exec --no-home {bindings_str} {simg} \
xnat2bids {' '.join(args[0])}")

# Define bids root directory
if(bids_root == ""):
bids_root = f"{data_dir}/shared/bids-export/{user}"
# Set logging level per session verbosity.
setLoggingLevel(args[0])

# Define singularity image
simg=f"/gpfs/data/bnc/simgs/brownbnc/xnat-tools-{version}.sif"
logging.debug({
"message": "Executing xnat2bids",
"session": args[0][0],
"command": srun_cmd
})

# Run xnat2bids asynchronously
task = asyncio.create_task(asyncio.create_subprocess_exec(*srun_cmd))
tasks.append(task)

# Run xnat2bids via srun
subprocess.run(shlex.split(
f"srun -t {time} --mem {mem} -N {nodes} -c {cpus} -J {job} \
-o {output} --mail-type=ALL --mail-user {email} \
singularity exec --contain -B {data_dir} {simg} \
xnat2bids {session} {bids_root} -u {user} -p {password}"
))
# Wait for all subprocess tasks to complete
await asyncio.gather(*tasks)

logging.info("Launched %d %s", len(tasks), "jobs" if len(tasks) > 1 else "job")
logging.info("Processed Scans Located At: %s", bids_root)

if __name__ == "__main__":
asyncio.run(main())
48 changes: 48 additions & 0 deletions tests/test_xnat2bids.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from toml import load
fordmcdonald marked this conversation as resolved.
Show resolved Hide resolved
import os
import sys
sys.path.append(os.path.abspath("../"))
from run_xnat2bids import compileArgumentList

def test_xnat2bids():
# Load test parameters
test_params = load("/gpfs/data/bnc/shared/scripts/fmcdona4/oscar-scripts/tests/x2b_test_config.toml")
user = "test_user"
session = test_params['xnat2bids-args']['sessions'][0]

# Initialize validation sets
x2b_validation_set = [
'XNAT_E00152',
'/gpfs/data/bnc/shared/bids-export/',
'--host https://xnat.bnc.brown.edu',
'--session-suffix -1',
'--bidsmap-file /gpfs/data/bnc/shared/scripts/fmcdona4/bidsmap.json',
'--includeseq 1 --includeseq 2',
'--skipseq 3',
'--overwrite']


slurm_validation_set = [
'--time 04:00:00',
'--mem 16000',
'--nodes 1',
'--cpus-per-task 2',
'--job-name xnat2bids',
'--output /gpfs/scratch/%u/logs/xnat2bids_test%J.log',
'--mail-user [email protected]',
'--mail-type ALL']

bindings_validation_set = [
"/gpfs/data/bnc/shared/bids-export/",
"/gpfs/data/bnc/shared/scripts/fmcdona4/bidsmap.json"
]

# Run compileArgumentList()
x2b_param_list, slurm_param_list, bindings = compileArgumentList(session, test_params, user)
assert x2b_param_list == x2b_validation_set
assert slurm_param_list == slurm_validation_set
assert bindings == bindings_validation_set

if __name__ == "__main__":
test_xnat2bids()

21 changes: 21 additions & 0 deletions tests/x2b_test_config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[slurm-args]
Copy link
Contributor

Choose a reason for hiding this comment

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

commit the values that we have good defaults for

time = "04:00:00"
mem = 16000
nodes = 1
cpus-per-task = 2
job-name = "xnat2bids"
output = "/gpfs/scratch/%u/logs/xnat2bids_test%J.log"
mail-user = "[email protected]"
mail-type = "ALL"

[xnat2bids-args]
sessions = ["XNAT_E00152"]
bids_root = "/gpfs/data/bnc/shared/bids-export/"
version = ""
host="https://xnat.bnc.brown.edu"
session-suffix=-1
bidsmap-file="/gpfs/data/bnc/shared/scripts/fmcdona4/bidsmap.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

includeseq=[1,2]
skipseq=[3]
verbose=0
overwrite=true
14 changes: 0 additions & 14 deletions x2b_config.toml

This file was deleted.

10 changes: 10 additions & 0 deletions x2b_default_config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[slurm-args]
time = "04:00:00"
mem = 16000
nodes = 1
cpus-per-task = 2
job-name = "xnat2bids"
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@fordmcdonald fordmcdonald Apr 3, 2023

Choose a reason for hiding this comment

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

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 ?


[xnat2bids-args]
host="https://xnat.bnc.brown.edu"

Loading