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

LinalgRefactor - Apply/MatrixProd #3584

Merged
merged 1 commit into from Feb 9, 2017

Conversation

Projects
None yet
2 participants
@OXPHOS
Member

OXPHOS commented Dec 20, 2016

No description provided.

@karlnapf

This comment has been minimized.

Show comment
Hide comment
@karlnapf

karlnapf Dec 21, 2016

Member

BTW: check out git commit -amend

Member

karlnapf commented Dec 21, 2016

BTW: check out git commit -amend

@karlnapf

This comment has been minimized.

Show comment
Hide comment
@karlnapf

karlnapf Jan 23, 2017

Member

What is the state here now, I cannot access the old discussion anymore. ...

Member

karlnapf commented Jan 23, 2017

What is the state here now, I cannot access the old discussion anymore. ...

@OXPHOS

This comment has been minimized.

Show comment
Hide comment
@OXPHOS

OXPHOS Jan 30, 2017

Member

@karlnapf We were saying renaming the mat * vec method and you proposed dot. However now dot is vec * vec method, which returns scalar type T. I don't think it can be overloaded for mat * vec method. I have thought about making mat * mat and mat * vec one method such as matrix_prod, but I don't think SGVector and SGMatrix are 100% interchangable.

Member

OXPHOS commented Jan 30, 2017

@karlnapf We were saying renaming the mat * vec method and you proposed dot. However now dot is vec * vec method, which returns scalar type T. I don't think it can be overloaded for mat * vec method. I have thought about making mat * mat and mat * vec one method such as matrix_prod, but I don't think SGVector and SGMatrix are 100% interchangable.

@karlnapf

This comment has been minimized.

Show comment
Hide comment
@karlnapf

karlnapf Jan 30, 2017

Member

I see, then we need different names for each. There is really no way of overloading mat-vec and mat-mat? Calling it mat_mul ?

Maybe look out in other libs for some inspiration on consistent naming ...

Member

karlnapf commented Jan 30, 2017

I see, then we need different names for each. There is really no way of overloading mat-vec and mat-mat? Calling it mat_mul ?

Maybe look out in other libs for some inspiration on consistent naming ...

@OXPHOS

This comment has been minimized.

Show comment
Hide comment
@OXPHOS

OXPHOS Feb 6, 2017

Member

@karlnapf I was being stupid..so I merged mat * vec to mat * mat by overloading matrix_prod method.

Member

OXPHOS commented Feb 6, 2017

@karlnapf I was being stupid..so I merged mat * vec to mat * mat by overloading matrix_prod method.

@OXPHOS OXPHOS changed the title from LinalgRefactor - Apply to LinalgRefactor - Apply/MatrixProd Feb 7, 2017

float64_t ref[] = {10, 11.5, 13, 14.5};
EXPECT_EQ(x.vlen, A.num_rows);

This comment has been minimized.

@karlnapf

karlnapf Feb 9, 2017

Member

this needs to be ASSERT_EQUAL technically otherwise the loop below can segfault, same below. But nevermind

@karlnapf

karlnapf Feb 9, 2017

Member

this needs to be ASSERT_EQUAL technically otherwise the loop below can segfault, same below. But nevermind

@karlnapf

This comment has been minimized.

Show comment
Hide comment
@karlnapf

karlnapf Feb 9, 2017

Member

Its good to go! Nice!

Member

karlnapf commented Feb 9, 2017

Its good to go! Nice!

@karlnapf karlnapf merged commit afadf98 into shogun-toolbox:feature/linalg_refactor Feb 9, 2017

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default DEV build done.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment