Skip to content

Quick Fix for issues arising with step size in sequences > 1#1847

Merged
umar456 merged 1 commit into
arrayfire:develfrom
pavanky:index_step_fix
Jun 22, 2017
Merged

Quick Fix for issues arising with step size in sequences > 1#1847
umar456 merged 1 commit into
arrayfire:develfrom
pavanky:index_step_fix

Conversation

@pavanky

@pavanky pavanky commented Jun 21, 2017

Copy link
Copy Markdown
Member

@umar456 umar456 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. 👍 Minor changes requested.

Comment thread test/index.cpp Outdated
ASSERT_EQ(af::allTrue<bool>(c == d), true);
}

TEST(Index, ISSUE_184_Index_Step_reorder)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ISSUE_1845_Index_Step_reorder

Comment thread src/backend/opencl/Array.cpp Outdated
dim4 parent_strides = parent.strides();

if (dStrides != parent_strides) {
const Array<T> &parentCopy = copyArray(parent);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

copyArray returns an Array by value. I would just make this a value Array rather than a reference array. The createSubArray function should increase the reference count anyway. It would be a good idea to check.

Comment thread test/index.cpp Outdated
af::array b = a(span, af::seq(0, af::end, 2));
af::array c = b(span, af::seq(0, af::end, 3));
af::array d = a(span, af::seq(0, af::end, 6));
ASSERT_EQ(af::allTrue<bool>(c == d), true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

EXPECT_TRUE(af::allTrue<bool>(c == d));

Comment thread test/index.cpp Outdated
array a = af::randu(1,8,1);
array b = reorder(a,0,2,1);
array d = reorder(b(0,0,span),2,1,0);
ASSERT_EQ(af::allTrue<bool>(a.T() == d), true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

EXPECT_TRUE(af::allTrue<bool>(a.T() == d));

Comment thread test/index.cpp Outdated
{
using namespace af;
af::array a = af::randu(3, 12);
af::array b = a(span, af::seq(0, af::end, 2));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

af:: is unnecessary.

- Added relevant tests
- Updated release notes
@umar456 umar456 merged commit a4854b5 into arrayfire:devel Jun 22, 2017
@pavanky pavanky deleted the index_step_fix branch June 22, 2017 03:33
@mlloreda mlloreda added this to the v3.5.0 milestone Jul 12, 2017
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.

3 participants