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

Bug/591 slicing memory issues #594

Merged
merged 8 commits into from
Jun 15, 2020
Merged

Conversation

coquelin77
Copy link
Member

Description

getitem/setitem now use tuples for most key types.
Advanced indexing is also implemented via this approach. keys for advanced indexing can be lists, torch.Tensors, or ht.DNDarrays. This means that the results of where and nonzero can be fed directly into another DNDarray as a key. However, it also means that lists are treated differently than tuples when given as a key

Issue/s resolved: #591 #505

Changes proposed:

  • most keys in getitem/setitem create a tuple for a given key
  • removal of other types of key options in getitem/setitem
  • reduce_op neutral value array created using the gshape instead of the lshape
  • addition of advanced indexing

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Due Diligence

  • All split configurations tested
  • Multiple dtypes tested in relevant functions
  • Documentation updated (if needed)
  • Updated changelog.md under the title "Pending Additions"

Does this change modify the behaviour of other functions? If so, which?

yes, any function which uses where or nonzero can now use the DNDarray instead of a list.

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #594 into master will increase coverage by 0.01%.
The diff coverage is 97.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #594      +/-   ##
==========================================
+ Coverage   96.47%   96.48%   +0.01%     
==========================================
  Files          75       75              
  Lines       15169    15214      +45     
==========================================
+ Hits        14634    14679      +45     
  Misses        535      535              
Impacted Files Coverage Δ
heat/core/dndarray.py 96.90% <97.09%> (+0.05%) ⬆️
heat/core/arithmetics.py 99.19% <100.00%> (ø)
heat/core/indexing.py 100.00% <100.00%> (ø)
heat/core/manipulations.py 99.51% <100.00%> (ø)
heat/core/operations.py 92.81% <100.00%> (+0.03%) ⬆️
heat/core/statistics.py 97.82% <100.00%> (ø)
heat/core/stride_tricks.py 100.00% <100.00%> (ø)
heat/core/tests/test_arithmetics.py 97.88% <100.00%> (+<0.01%) ⬆️
heat/core/tests/test_dndarray.py 99.37% <100.00%> (+<0.01%) ⬆️
heat/core/tests/test_indexing.py 92.75% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc192a0...ca7ceab. Read the comment docs.

Copy link
Member

@Markus-Goetz Markus-Goetz left a comment

Choose a reason for hiding this comment

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

Mostly minor changes. A job well done. Thanks Daniel

heat/core/dndarray.py Outdated Show resolved Hide resolved
heat/core/dndarray.py Show resolved Hide resolved
heat/core/dndarray.py Outdated Show resolved Hide resolved
heat/core/dndarray.py Outdated Show resolved Hide resolved
heat/core/dndarray.py Show resolved Hide resolved
heat/core/stride_tricks.py Show resolved Hide resolved
heat/core/stride_tricks.py Show resolved Hide resolved
heat/core/stride_tricks.py Show resolved Hide resolved
heat/core/tests/test_dndarray.py Outdated Show resolved Hide resolved
heat/core/tests/test_dndarray.py Outdated Show resolved Hide resolved
@Markus-Goetz Markus-Goetz merged commit 9108523 into master Jun 15, 2020
@Markus-Goetz Markus-Goetz deleted the bug/591-slicing_memory_issues branch June 15, 2020 10:59
@Markus-Goetz Markus-Goetz mentioned this pull request Jun 15, 2020
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.

High memory usage of slices
2 participants