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

Adds engine:serial default #1141

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Adds engine:serial default #1141

merged 1 commit into from
Jun 4, 2024

Conversation

teresamg
Copy link
Contributor

@teresamg teresamg commented Jun 3, 2024

No description provided.

@pep8speaks
Copy link

Hello @teresamg! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 165:9: E303 too many blank lines (2)

@arokem
Copy link
Collaborator

arokem commented Jun 3, 2024

Thanks for adding this! IIUC, this is a followup to #1124?

Could you say a little about the errors that you are seeing without the addition of this line?

@teresamg
Copy link
Contributor Author

teresamg commented Jun 3, 2024

GroupAFQ's parallel_params defaults to engine: serial, and this should still happen when using ParallelGroupAFQ. If there were errors with this line we can try something else?

@arokem
Copy link
Collaborator

arokem commented Jun 3, 2024

Yeah, I think for now users would have to add: "engine" : "serial" to their code, along the lines of:

import os.path as op

from AFQ.api.group import ParallelGroupAFQ
from AFQ.definitions.image import RoiImage
import AFQ.api.bundle_dict as abd
import AFQ.data.fetch as afd


_, bids_path = afd.fetch_hbn_preproc(
        ["NDARZT957CWG",
         "NDARZU279XR3",
         "NDARZU401RCU",
         "NDARZU822WN3",
         "NDARZV421TCZ",
         "NDARZW262ZLV",
         "NDARZX163EWC",
         ],
        path="/gscratch/scrubbed/arokem/data/")
    
my_afq = ParallelGroupAFQ(
    bids_path=bids_path,
    preproc_pipeline="qsiprep",
    parallel_params={
        "engine" : "serial",
        "submitter_params": {
            "plugin": "slurm",
            "sbatch_args": "-J test \
                            -p ckpt \
                            --nodes=1 \
                            --cpus-per-task=8 \
                            --gpus=1 \
                            -A escience \
                            --mem=64G \
                            --time=2:00:00 \
                            -o /gscratch/scrubbed/arokem/logs/test.out \
                            -e /gscratch/scrubbed/arokem/logs/test.err \
                            --mail-user=arokem@uw.edu \
                            --mail-type=ALL"
        },
        "cache_dir": "/gscratch/scrubbed/arokem/tmp"
    }
)

my_afq.export_all()

Is that correct?

@teresamg
Copy link
Contributor Author

teresamg commented Jun 3, 2024

It should run, but users won't always know to specify engine: serial, and don't have to unless specifying other parameters, so I think the default should be in there somehow!

@36000
Copy link
Collaborator

36000 commented Jun 4, 2024

This makes sense to me! Merging

@36000 36000 merged commit 5cbe265 into yeatmanlab:master Jun 4, 2024
9 checks passed
@teresamg teresamg deleted the pydra branch June 6, 2024 20:48
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.

4 participants