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

Add arg to bids command for absolute path to BIDS root #390

Open
alyssadai opened this issue Nov 15, 2024 · 2 comments
Open

Add arg to bids command for absolute path to BIDS root #390

alyssadai opened this issue Nov 15, 2024 · 2 comments

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Nov 15, 2024

BIDS session paths are currently resolved using the absolute path of the BIDS directory (via the --bids-dir arg) inside a Docker container. This means that it is not guaranteed that the session paths derived from the within-container BIDS root actually correspond to their real paths on the host FS where the CLI was run. Rather, this depends on whether the user has mounted the BIDS directory to the same absolute location in the CLI container as the host (the example commands provided in our docs do this by default).

More experienced Docker users (e.g., our Nipoppy friends) may choose to mount paths differently when running the CLI, resulting in session paths in the JSONLD file that do not actually exist in the source FS where the dataset is stored.

Goal

Make it possible to derive the absolute path of the BIDS directory root on the source filesystem (not the container filesystem) based on CLI inputs, regardless of the exact volume mounting used by the user.

Possible solutions

  1. Accept original absolute path of BIDS directory root as required input, e.g.:
docker run --rm --volume=$PWD:/bids -w /bids neurobagel/bagelcli bids \
    --jsonld-path "Dataset1.jsonld" \
    --bids-dir "/bids" \  # Path inside the container
    --host-bids-dir "/host/absolute/path/to/bids" \  # Absolute path on the host
    --output "Dataset1.jsonld" \

We can then use --bids-dir to resolve the actual BIDS structure and extract session paths based on the mounted directory, but construct the absolute 'source' session paths for subjects' JSONLD data using --host-bids-dir (which is independent of the volume mounting)

  1. Pass original absolute path of BIDS directory via a required environment variable, e.g.:
docker run --rm \
    --env HOST_BIDS_DIR=$PWD/dataset1 \
    --volume=$PWD:/bids -w /bids neurobagel/bagelcli bids \
    --jsonld-path "Dataset1.jsonld" \
    --bids-dir "/bids" \
    --output "Dataset1.jsonld" \
    --overwrite

Again, we can reconstruct the absolute source BIDS paths using HOST_BIDS_DIR.

@alyssadai alyssadai added the flag:schedule Flag issue that should go on the roadmap or backlog. label Nov 15, 2024
@alyssadai alyssadai changed the title Add arg to bids command for absolute path to BIDS root Add arg to bids command for absolute path to BIDS root Nov 16, 2024
@surchs surchs moved this to Backlog in Neurobagel Nov 18, 2024
@surchs surchs removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Nov 18, 2024
@surchs surchs moved this from Backlog to Specify - Active in Neurobagel Dec 14, 2024
@alyssadai
Copy link
Contributor Author

@surchs, mind taking a look at the description and proposed solutions and seeing if either makes sense to you?

@surchs
Copy link
Contributor

surchs commented Dec 18, 2024

I like option 1 better. But that gives us 4 things a user needs to control regarding BIDS:

  1. host-bids: The (absolute or relative) path on the host FS to the BIDS root (i.e. what goes on the left of the volume mount)
  2. container-bids: The absolute path where the BIDS root are mounted in the container
  3. --bids-dir: the input argument for where to find the BIDS root inside the container
  4. (new) real-bids: the absolute path where a user should go look for the BIDS root folder (may be different than host-bids
    How about we make the internal mount point container-bids fixed and make `

That's a lot of similar things. So I would suggest we

  • fix container-bids (e.g. always mount into /bids) and
  • make the --bids-dir flag optional and default to the work_dir in the container. We'd use your -w /bids and bake it into the image
  • make a custom entrypoint to the container that checks if /bids exists / has been mounted to and prints the correct docker invocation if not.

This still allows the CLI to run in "bare-metal" mode, but for most cases (i.e. as a docker container) I need to put a lot less stuff / repeat myself a lot less.

So the new invocation would be

docker run --rm --volume=$PWD:/bids neurobagel/bagelcli bids \
    --jsonld-path "Dataset1.jsonld" \
    --host-bids-dir "/host/absolute/path/to/bids" \  # Absolute path on the host
    --output "Dataset1.jsonld" \

wdyt?

@rmanaem rmanaem moved this from Specify - Active to Specify - Done in Neurobagel Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Specify - Done
Development

No branches or pull requests

2 participants