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 iterable behavior for PointerTensor #3659

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

gmuraru
Copy link
Member

@gmuraru gmuraru commented Jun 1, 2020

Pull Request

Fixes #3543

Description

Add the possibility to do:

for tensor in ptr:
     print(tensor) # should return a new pointer tensor

Affected Dependencies

List any dependencies that are required for this change.

Type of Change

Please mark options that are relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation (non-breaking change which adds documentation)
  • Improvement (non-breaking change that improves the performance or reliability of existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How has this been tested?

  • Test if the number of tensors alice has
  • Provide instructions so we can reproduce.
  • List any relevant details for your test configuration.

Checklist

Additional Context

Add a new method to the storage object such that we can call len(alice.object_store and will return the number of objects from the worker.

@gmuraru gmuraru requested a review from a team as a code owner June 1, 2020 19:27
Copy link
Contributor

@karlhigley karlhigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@karlhigley karlhigley added the Type: New Feature ➕ Introduction of a completely new addition to the codebase label Jun 1, 2020
@codecov
Copy link

codecov bot commented Jun 1, 2020

Codecov Report

Merging #3659 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3659   +/-   ##
=======================================
  Coverage   94.74%   94.75%           
=======================================
  Files         182      182           
  Lines       18036    18061   +25     
=======================================
+ Hits        17089    17114   +25     
  Misses        947      947           
Impacted Files Coverage Δ
test/generic/pointers/test_callable_pointer.py 100.00% <ø> (ø)
test/generic/pointers/test_dataset_pointer.py 100.00% <ø> (ø)
test/generic/pointers/test_multi_pointer.py 100.00% <ø> (ø)
test/generic/pointers/test_pointer_plan.py 100.00% <ø> (ø)
test/generic/test_autograd.py 100.00% <ø> (ø)
test/generic/test_functions.py 100.00% <ø> (ø)
test/generic/test_gc.py 100.00% <ø> (ø)
test/generic/test_logging.py 100.00% <ø> (ø)
test/generic/test_private.py 100.00% <ø> (ø)
syft/generic/frameworks/hook/hook_args.py 95.49% <100.00%> (ø)
... and 13 more

@@ -161,3 +161,9 @@ def register_tags(self, obj):

for tag in obj.tags:
self._tag_to_object_ids[tag].add(obj.id)

def __len__(self):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@karlhigley are you also okay with this change? to be able to call len on the object store

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, took me a minute to see what it was for, but seems pretty convenient. I think there's a count_objects() method on BaseWorker that could use this, but that's a "nice to have tweak" that shouldn't block this PR.

@karlhigley karlhigley merged commit e794bc1 into OpenMined:master Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: New Feature ➕ Introduction of a completely new addition to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Iterate over wrapper of tensor pointer failed
2 participants