Implementation of proposed syntax changes #768

tclose · 2025-02-17T02:40:06Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Summary

Implements proposed syntax changes in #692

Test modules (to fix)

Checklist

I have added tests to cover my changes (if necessary)
I have updated documentation (if necessary)

Develop: fixes for test_environments

satra · 2025-02-25T13:44:57Z

@tclose - at this point, does it make sense to still keep two separate areas (design/engine) in the codebase? or simply merge them? i think we are all in agreement that this new design is the path we want to take.

tclose · 2025-02-25T21:05:56Z

@tclose - at this point, does it make sense to still keep two separate areas (design/engine) in the codebase? or simply merge them? i think we are all in agreement that this new design is the path we want to take.

Thanks :) although my plan was to keep the design area separate in the final state. Actually, in the refactor I had in mind I was planning to pull out environments and workers into separate namespaces, e.g. pydra.workers, pydra.environments, and have design, tasks, workers and environments (do we like plural or non-plural, i.e. task, worker, environment?) as expansion points where plugins can add support for new task-types, tasks, workers and environment classes.

I also wanted to pull out the helper functions into pydra.utils to avoid some circular import issues, leaving the only user-facing API within pydra.engine the Submitter class

…ere being included

Develop: Test specs

…s it is no longer necessary

Test_submitter

tclose · 2025-03-31T08:08:34Z

Ok, I believe this is all good to go!! I have added a brief glossary to the docs explaining some of the new concepts and a few of the attributes I renamed (see docs/source/reference/glossary.rst).

Let me know if there is anything else, otherwise it would be great to get this merged and a 1.0a released so I can start updating the rest of my packages to point to that 😁

satra · 2025-03-31T16:13:43Z

thanks tom. i will leave it to you and others to pull the trigger.

regarding prereleases and auto (which we use), we may need to setup a few things: https://intuit.github.io/auto/docs/generated/next (may not be a bad thing to setup).

alternatively, we could merge into master (should we rename to main) and make a manual pre-release.

pydra/environments/native.py

satra · 2025-03-31T16:23:43Z

one last question is the hashing of arbitrary objects, especially given the issues we had with torch arrays. is there a default serializer that should handle any arbitrary object? it did seem from that issue that we had to write and register our own serializer, and that the default backstop was not sufficient. (see: https://github.com/sensein/senselab/blob/5f1af5b5305579c8bdd698837dc8048d5284229d/src/senselab/utils/data_structures/pydra_helpers.py#L17).

tclose · 2025-03-31T23:39:39Z

one last question is the hashing of arbitrary objects, especially given the issues we had with torch arrays. is there a default serializer that should handle any arbitrary object? it did seem from that issue that we had to write and register our own serializer, and that the default backstop was not sufficient. (see: https://github.com/sensein/senselab/blob/5f1af5b5305579c8bdd698837dc8048d5284229d/src/senselab/utils/data_structures/pydra_helpers.py#L17).

The backstop object bytes-repr is here

pydra/pydra/utils/hash.py

Lines 308 to 346 in bc6b0a9

    
           @singledispatch 
        
           def bytes_repr(obj: object, cache: Cache) -> Iterator[bytes]: 
        
               """Default implementation of hashing for generic objects. Single dispatch is used 
        
               to provide hooks for class-specific implementations 
        
               Parameters 
        
               ---------- 
        
               obj: object 
        
                   the object to hash 
        
               cache : Cache 
        
                   a dictionary object used to store a cache of previously cached objects to 
        
                   handle circular object references 
        
               Yields 
        
               ------- 
        
               bytes 
        
                   unique representation of the object in a series of bytes 
        
               """ 
        
               cls = obj.__class__ 
        
               yield f"{cls.__module__}.{cls.__name__}:{{".encode() 
        
               dct: Dict[str, ty.Any] 
        
               if attrs.has(type(obj)): 
        
                   # Drop any attributes that aren't used in comparisons by default 
        
                   dct = attrs.asdict(obj, recurse=False, filter=lambda a, _: bool(a.eq)) 
        
               elif hasattr(obj, "__slots__") and obj.__slots__ is not None: 
        
                   dct = {attr: getattr(obj, attr) for attr in obj.__slots__} 
        
               else: 
        
                   def is_special_or_method(n: str): 
        
                       return (n.startswith("__") and n.endswith("__")) or inspect.ismethod( 
        
                           getattr(obj, n) 
        
                       ) 
        
                   try: 
        
                       dct = {n: v for n, v in obj.__dict__.items() if not is_special_or_method(n)} 
        
                   except AttributeError: 
        
                       dct = {n: getattr(obj, n) for n in dir(obj) if not is_special_or_method(n)} 
        
               yield from bytes_repr_mapping_contents(dct, cache) 
        
               yield b"}"

I did make some changes to it so that it can handle slots classes (as well as special handling for attrs-generated objects), so would be interested if it can now handle the PyTorch arrays. If not, I might be worth adding those bytes-repr functions to the hash module with guards for the case where torch is not installed

satra · 2025-03-3 6D40 1T23:55:07Z

perhaps i'm doing something wrong, but let me know if this makes sense to you. it doesn't seem to be reflecting the object fully, i.e. the output doesn't change for the two different tensors.

In [15]: a = torch.tensor([1, 2])

In [16]: list(bytes_repr(a, cache=cache))
Out[16]: [b'torch.Tensor:{', b'}']

In [17]: a = torch.tensor([3, 4])

In [18]: list(bytes_repr(a, cache=cache))
Out[18]: [b'torch.Tensor:{', b'}']

for a backstop i was hoping it would be similar to the cloudpickle/pickle option, where as long as it is pickleable we can potentially hash it.

tclose · 2025-03-31T23:55:29Z

https://intuit.github.io/auto/docs/generated/next

I had a look at the shipit pre-release docs, and I think that would work better > 1.0, as we don't really want to keep the current master as the default until 1.0 is released.

So if you guys are ok with it, I will pull the trigger on the merge and manually release a 1.0a. I'm also happy to rename the master branch to main in the process if you like.

satra · 2025-03-31T23:58:33Z

whereas the bytes_repr works fine for numpy:

In [21]: a = np.array([3, 4])

In [22]: list(bytes_repr(a, cache=cache))
Out[22]: 
[b'numpyndarray:2:',
 b'\x03\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00']

In [23]: a = np.array([1, 2])

In [24]: list(bytes_repr(a, cache=cache))
Out[24]: 
[b'numpyndarray:2:',
 b'\x01\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00']

tclose · 2025-04-01T00:16:22Z

perhaps i'm doing something wrong, but let me know if this makes sense to you. it doesn't seem to be reflecting the object fully, i.e. the output doesn't change for the two different tensors.
In [15]: a = torch.tensor([1, 2])

In [16]: list(bytes_repr(a, cache=cache))
Out[16]: [b'torch.Tensor:{', b'}']

In [17]: a = torch.tensor([3, 4])

In [18]: list(bytes_repr(a, cache=cache))
Out[18]: [b'torch.Tensor:{', b'}']
for a backstop i was hoping it would be similar to the cloudpickle/pickle option, where as long as it is pickleable we can potentially hash it.

Interesting, torch.Tensor doesn't have slots and its __dict__ is empty. We could just fallback to dir/getattr, although we will need to enclose it in try/except to catch invalid properties (I'm wondering whether we could just check set(dir(obj)) - set(dir(type(obj))) or whether that would miss things.

Cloudpickle would be guaranteed to be unique, but it is not guaranteed to produce the same representations between equivalent objects so you would get a lot of false-negative cache lookups, e.g.

import torch
import cloudpickle as cp

a = torch.tensor([1, 2])
cp1 = cp.dumps(a)

b = torch.tensor([1, 2])
cp2 = cp.dumps(b)

print(cp1 == cp2)  # False

tclose · 2025-04-01T00:17:34Z

perhaps i'm doing something wrong, but let me know if this makes sense to you. it doesn't seem to be reflecting the object fully, i.e. the output doesn't change for the two different tensors.
In [15]: a = torch.tensor([1, 2])

In [16]: list(bytes_repr(a, cache=cache))
Out[16]: [b'torch.Tensor:{', b'}']

In [17]: a = torch.tensor([3, 4])

In [18]: list(bytes_repr(a, cache=cache))
Out[18]: [b'torch.Tensor:{', b'}']
for a backstop i was hoping it would be similar to the cloudpickle/pickle option, where as long as it is pickleable we can potentially hash it.
Interesting, torch.Tensor doesn't have slots and its __dict__ is empty. We could just fallback to dir/getattr, although we will need to enclose it in try/except to catch invalid properties (I'm wondering whether we could just check set(dir(obj)) - set(dir(type(obj))) or whether that would miss things.

Cloudpickle would be guaranteed to be unique, but it is not guaranteed to produce the same representations between equivalent objects so you would get a lot of false-negative cache lookups, e.g.
import torch
import cloudpickle as cp

a = torch.tensor([1, 2])
cp1 = cp.dumps(a)

b = torch.tensor([1, 2])
cp2 = cp.dumps(b)

print(cp1 == cp2)  # False

Maybe this could be addressed in a separate PR though...

satra · 2025-04-01T00:33:03Z

thanks for the cloudpickle example - fine with addressing this in a separate PR.

satra · 2025-04-01T01:04:53Z

woohoo!

This was referenced Feb 17, 2025

Draft implementation of proposed syntax changes (feedback requested) #766

Closed

Develop: fixes for test_environments #769

Merged

tclose added 18 commits February 25, 2025 09:27

working on singularity errors

f1a6cb3

more debugging of singularity

d8696f2

more debugging

b30d982

attempting to fix singularity tests

bc5397d

fixed up environments import

b1184d4

shell.define can take a list of tokens as well as a single string

c8bdd3b

debugging docker unittests

2fc135f

test_dockertask tests pass

a42cc51

fixed initialisation of environment in Submitter

fdd5e52

debugged test_dockertask

44d0d67

fixed merge error

28f1bc5

fixed merging error in test_shell

3f397e0

debugging unittests

4080298

debugged singularity tests

ffeece0

made imports in pydra.design.* modules absolute

3b8f818

added fileformats-extras to test deps

6aaf1b7

fetch tags in github actions so installed version is correct

e6724c1

Merge pull request #769 from nipype/test_environments

4b97ed6

Develop: fixes for test_environments

tclose added 8 commits February 26, 2025 14:26

fixed bug in state depth calculation where, single, combined states w…

f7021e5

…ere being included

moved resolved lazy inputs into NodeExecution class from TaskDef

b2034d5

finished debugging test_specs

db8f799

Merge pull request #770 from nipype/test_specs

b5a0455

Develop: Test specs

debugging test_specs

33bb999

deleted workflow to pick up changes in workflow graph error message a…

bb75d94

…s it is no longer necessary

Merge pull request #771 from nipype/test_submitter

a5f6e45

Test_submitter

debugging test_task

bda7ea2

tclose added 4 commits March 31, 2025 17:36

renamed cache_locations to readonly_caches

c538929

renamed cont_dim to container_ndim

89a474a

renamed additional_args to append_args

ba08ee9

renamed output_dir to cache_dir

ac19f96

tclose force-pushed the develop branch from d4bd405 to ac19f96 Compare March 31, 2025 07:07

tclose added 3 commits March 31, 2025 18:22

cleaning up other references to output_dir

dbf4bb1

expanded glossary

b148052

finished glossary

1c49336

satra reviewed Mar 31, 2025

View reviewed changes

pydra/environments/native.py Outdated Show resolved Hide resolved

tclose added 3 commits April 1, 2025 08:49

added splitter and combiner to glossary

15a0fc1

added link to glossary on index page

4ae54a7

changed Native.__doc__

bc6b0a9

satra mentioned this pull request Apr 1, 2025

Task outputting Torch.Tensor now errors (reports having no out attribute) #767

Open

tclose merged commit a41d30d into master Apr 1, 2025
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of proposed syntax changes #768

Implementation of proposed syntax changes #768

Implementation of proposed syntax changes #768

Implementation of proposed syntax changes #768

Conversation

Types of changes

Summary

Test modules (to fix)

Checklist