Workflow crashing with file not found due to strange nipype.pipeline.engine.utils.modify_path behavior

[stilley2]

Summary

When getting the node results, the modify_path function will mistakenly identify some outputs as files, causing the run to fail.

Actual behavior

Workflow crashes with a file not found error due to output being mistakenly identified as a file. This occurs when the output is a string (or bytes? untested), and a file with the same name exists in the current working directory.

Expected behavior

modify_path does not identify the object as a file.

How to replicate the behavior

See below

Script/Workflow details

The following script will fail if there is a file named "22" in the directory from which it is called.

from nipype.pipeline import engine as pe
from nipype.interfaces.base import BaseInterfaceInputSpec, TraitedSpec, BaseInterface, traits


class Concat2InputSpec(BaseInterfaceInputSpec):
    x = traits.String()


class Concat2OutputSpec(TraitedSpec):
    y = traits.String()


class Concat2(BaseInterface):
    input_spec = Concat2InputSpec
    output_spec = Concat2OutputSpec

    def _run_interface(self, runtime):
        self._outval = self.inputs.x + '2'
        return runtime

    def _list_outputs(self):
        outputs = self._outputs().get()
        outputs['y'] = self._outval
        return outputs


if __name__ == '__main__':
    node = pe.Node(Concat2(x=2), 'double')
    node.run()

Please put URL to code or code here (if not too long).

Platform details:

{'commit_hash': '7fbc7cbac',
 'commit_source': 'repository',
 'networkx_version': '2.3',
 'nibabel_version': '2.4.1',
 'nipype_version': '1.2.1-dev+g7fbc7cbac',
 'numpy_version': '1.16.4',
 'pkg_path': '/home/stilley2/dev/nipype_debug/nipype/nipype',
 'scipy_version': '1.3.0',
 'sys_executable': '/home/stilley2/dev/nipype_debug/pyenv/bin/python',
 'sys_platform': 'linux',
 'sys_version': '3.7.3 (default, May 11 2019, 00:38:04) \n'
                '[GCC 9.1.1 20190503 (Red Hat 9.1.1-1)]',
 'traits_version': '5.1.1'}

Execution environment

• My python environment outside container

[effigies]

For added context, running the provided script produces the following error:

190618-19:12:45,52 nipype.workflow INFO:
	 [Node] Setting-up "double" in "/private/var/folders/yb/10x6vxlx40s1dhv5vc163m680000gp/T/tmpwhu6niza/double".
190618-19:12:45,57 nipype.workflow INFO:
	 [Node] Running "double" ("__main__.Concat2")
Traceback (most recent call last):
  File "test.py", line 29, in <module>
    node.run()
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 487, in run
    self, report_type='postexec', is_mapnode=isinstance(self, MapNode))
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/utils.py", line 149, in write_report
    result = node.result  # Locally cache result
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 197, in result
    return _load_resultfile(self.output_dir(), self.name)[0]
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/utils.py", line 359, in load_resultfile
    basedir=path).items()):
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/utils.py", line 478, in modify_paths
    val, relative=relative, basedir=basedir)
  File "/anaconda3/lib/python3.6/site-packages/nipype/pipeline/engine/utils.py", line 498, in modify_paths
    raise IOError('File %s not found' % out)
OSError: File /private/var/folders/yb/10x6vxlx40s1dhv5vc163m680000gp/T/tmpwhu6niza/double/22 not found

Further, this was introduced in #2325, due to the saving/loading of the resultfile. I suspect the fact that we're losing trait information is the problem. Still trying to work through whether the correct response is the fix proposed in #2945.

[effigies]

cc @oesteban you might see the way to go here better, since #2325 was your refactor.

[oesteban]

I'm not sure this is a problem of nipype - although I acknowledge there could be potential file name collisions conducive to this problem and we may want to keep track of existing files in the results file.

I'll wait on the original fMRIPrep issue to understand the problem.

[stilley2]

I'm pretty sure this is a nipype issue and not an fmriprep issue. The above example does not rely on fmriprep, and definitely results in undesirable behavior. I agree with @effigies that the main issue is loss of type information, and trying to recover that information, so #2945 is probably not an ideal solution.

[oesteban]

I can only imagine two situations where this is an issue:

• If an interface has both an string output and a file output, and both values coincide (which is a pretty particular condition, and I'm not sure there is an interface actually doing this).
• If a previous run of a workflow is manipulated adding such a file (which is artificial and this is why I'm not convinced this is a nipype issue).

For that reason, I think we should first replicate and identify the error condition in fMRIPrep and then decide whether it is worth tracking file traits.

WDYT @satra?

[stilley2]

The example script I posted doesn't satisfy either criteria, but still causes an issue. To cause this issue all that is required is:

1. an interface has a string output
2. A file with a name equal to that string exists in the directory from which the python script was called (i.e., not the workflow's working directory).

[oesteban]

Gotcha, I hadn't realized of number 2. Well, I guess then #2945 should address this issue.

[effigies]

One question I have is why we're modifying paths in the resultfile.

[oesteban]

It is a buggy filter to convert paths to relative (before storing) and then back to absolute (after loading).

[effigies]

Sure, but why?

[oesteban]

I would imagine (and that was there before my refactor) that it is to make the result file work even if you move the work directory somewhere else. For a single run (no reuse), I agree it doesn't make much sense. @satra may have a better understanding of this.

[satra]

@oesteban and @effigies - this should be controlled through a config option:

https://github.com/nipy/nipype/blob/master/nipype/utils/config.py#L65

but it sounds like the option is not being implemented properly. the default is false, which means it should not use relative paths unless forced.

[effigies]

I think that's being handled inside modify_paths:

nipype/nipype/pipeline/engine/utils.py

Lines 490 to 496 in 1eeabd3

	if relative:
	if config.getboolean('execution', 'use_relative_paths'):
	out = relpath(object, start=basedir)
	else:
	out = object
	else:
	out = os.path.abspath(os.path.join(basedir, object))

The problem is that we're identifying paths as strings that exist on the filesystem relative to some directory (either CWD or node-WD), which can sometimes be true for String traits, as well. So avoiding relative paths won't save us when the string output is a valid path; forcing to absolute will still modify the string.

[satra]

@effigies - in other situations we check the traits metadata to determine if this is a string vs a file. (hash_files=False). a good improvement would be to separate out a Path object that is a string vs a Path object that is a file.

[effigies]

in other situations we check the traits metadata to determine if this is a string vs a file. (hash_files=False).

Yes, but this is operating on a dict, without trait information. I'm working on a strategy to pass in traits as type hints.

a good improvement would be to separate out a Path object that is a string vs a Path object that is a file.

Could you clarify? Are you suggesting to use pathlib.Path as the object that is pickled?

[satra]

@effigies - i simply meant altering the traits_extension File class to either return a string or a pathlib object depending on the traits metadata.

[effigies]

@effigies - i simply meant altering the traits_extension File class to either return a string or a pathlib object depending on the traits metadata.

I've looked at the traits stuff, and have decided I don't really know how to start. Just noting this, in case somebody else has time to play with traits.

[oesteban]

Having a look right this minute

[oesteban]

@satra the new trait would return pathlike if exists is True or this new metadata is True, correct?

[satra]

@oesteban - the metadata already exists, so the validate function in the File trait would return a Path object if hash_files is not False

[satra]

i would start with updating the File/Directory trait in traits_extensiion instead of creating a new trait.

[oesteban]

Sorry, I was very unclear. Please check the draft PR above - I was actually modifying File/Directory.

[oesteban]

@satra, the metadata name is nohash, correct?

[satra]

the metadata name is hash_files, nohash is when a particular traits item should not be included in the hash computation. while hash_files either hashes the string (when False) or the content/timestamp (when True or missing)