search

nipreps / sdcflows

Susceptibility Distortion Correction (SDC) workflows for EPI MR schemes
https://www.nipreps.org/sdcflows
Apache License 2.0
31 stars 24 forks source link

B0FieldIdentifier: some characters should be replaced to set workflow name to avoid crashes in graph creation #295

Open bpinsard opened 1 year ago

bpinsard commented 1 year ago

FMRIPrep version 20.1.0 Not sure if that is in fMRIPrep or SDCFlows that the workflow name is set, but I get the following error if B0FieldIdentifier contains a - , and this will likely occur for other characters (I have no knowledge on dot files syntax though). 

  File "/opt/conda/lib/python3.8/site-packages/fmriprep/cli/run.py", line 77, in main
    fmriprep_wf.write_graph(graph2use="colored", format="svg", simple_form=True)
  File "/opt/conda/lib/python3.8/site-packages/nipype/pipeline/engine/workflows.py", line 465, in write_graph
    outfname = format_dot(dotfilename, format=format)
  File "/opt/conda/lib/python3.8/site-packages/nipype/pipeline/engine/utils.py", line 1404, in format_dot
    formatted_dot, _ = _run_dot(dotfilename, format_ext=format)
  File "/opt/conda/lib/python3.8/site-packages/nipype/pipeline/engine/utils.py", line 1420, in _run_dot
    res = CommandLine(cmd, terminal_output="allatonce", resource_monitor=False).run()
  File "/opt/conda/lib/python3.8/site-packages/nipype/interfaces/base/core.py", line 428, in run
    runtime = self._run_interface(runtime)
  File "/opt/conda/lib/python3.8/site-packages/nipype/interfaces/base/core.py", line 822, in _run_interface
    self.raise_exception(runtime)
  File "/opt/conda/lib/python3.8/site-packages/nipype/interfaces/base/core.py", line 749, in raise_exception
    raise RuntimeError(
RuntimeError: Command:
dot -Tsvg -o"...../workdir/fmriprep_wf/graph.svg" ".../workdir/fmriprep_wf/graph.dot"
Standard output:

Standard error:
Error: ..../workdir/fmriprep_wf/graph.dot: syntax error in line 29 near '-'

dot file contains something like fmriprep_wf_single_subject_01_wf_fmap_preproc_wf_in_b0id-test on the offending line.

bpinsard commented 1 year ago

Replaced offending characters in my B0Field* tags and it works now, if that can help anyone encountering that issue.

effigies commented 1 year ago

Ah, thanks for that. I think we assumed it would be a camel case string.

oesteban commented 8 months ago

We use the B0FieldIdentifier as an entity at the output (estimated fieldmaps). But the B0FieldIdentifier can definitely have hyphens, so we probably want to revise the workflow names and the datasinks.

effigies commented 3 months ago

This is turning out not to be resolvable purely in sdcflows. fMRIPrep at least (and thus probably nibabies and maybe dMRIPrep) use estimator.bids_id to both compare to B0FieldIdentifier and to look up workflow input/output spec names.

I think the fix, as @mgxd says in https://github.com/nipreps/sdcflows/pull/434#issuecomment-2141103753 is to make the sanitized ID available through the object API. We will then use that internally, and downstream tools need to transition.

If the sanitized ID replaces non-alphanumeric characters with _, then the workflows for currently working fieldmaps will be unchanged, and we could do this in a 2.8.x release. The workflows for non-working fieldmap identifiers will continue not to work, but at a different point of workflow construction.\

OTOH we would be introducing a new API feature, so it might make sense to call it 2.9.0, regardless.

WDYT? And do we care about getting this in for fMRIPrep 24.0, or should we leave it to the next round of releases?