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 in wrappers.py #30

Open
tgilbrough opened this issue Apr 20, 2016 · 22 comments
Open

Bug in wrappers.py #30

tgilbrough opened this issue Apr 20, 2016 · 22 comments
Labels

Comments

@tgilbrough
Copy link

tgilbrough commented Apr 20, 2016

Within the mat_vec function in the wrappers.py file, the out array is initialize to be the same size of input vector. Isn't this incorrect since the if we are multiplying a 3x2 matrix with a 2x1 vector, the output will be a 3x1 vector, not a 2x1 vector? Thanks!

@cswiercz
Copy link
Member

cswiercz commented Apr 21, 2016

Great observation and bug catch! Since I was only testing with square systems I never checked the case when dimensions were different. I will post an announcement on how to update your code. It should work like this:

  1. Add the homework2 repo as a remote, named "upstream" in this case:

    $ git remote add upstream https://github.com/uwhpsc-2016/homework2.git
    
  2. Pull the changes I will end up pushing to the upstream repo

    $ git pull upstream master
    

    (Commit your local changes before pulling.)

cswiercz added a commit to uwhpsc-2016/homework2 that referenced this issue Apr 21, 2016
Thank you to @tgilbrough for pointing this out.
See Issue uwhpsc-2016/syllabus#30
@cswiercz
Copy link
Member

@tgilbrough Actually, would you mind testing this for me before I email the entire class?

@tgilbrough
Copy link
Author

When I try and do the pull upstream, I am getting the following error:

[tgilbrou@f22 homework2-tgilbrough]$ git pull upstream master
remote: Repository not found.
fatal: repository 'https://github.com/uwpsc-2016/homework2.git/' not found

@cswiercz
Copy link
Member

cswiercz commented Apr 21, 2016

You misspelled "uwhpsc" in the URL. Use

$ git remote set-url upstream <newurl>

to fix it. See $ git help remote for details.

@ckchmod
Copy link

ckchmod commented Apr 21, 2016

git upstream works for me

@cswiercz
Copy link
Member

@shinwookang Awesome. Can you push the changes to your private remote so I can see?

@tgilbrough
Copy link
Author

Spelling things right makes quite the difference, thanks!

@cswiercz
Copy link
Member

@tgilbrough Excellent. Could you do the same as @shinwookang ?

@ckchmod
Copy link

ckchmod commented Apr 21, 2016

@cswiercz it's pushed. seems like it works.

@cswiercz
Copy link
Member

👍 I see the change reflected in your remote. Also, you didn't need to add a commit of your own. You could've just pushed the commit that I created to fix the issue.

@cswiercz
Copy link
Member

@tgilbrough I don't see my commit in your remote. Did you correctly pull the changes and apply them to your master branch before pushing?

@tgilbrough
Copy link
Author

I see the change too in my wrappers.py file so seemed to work perfectly.

@tgilbrough
Copy link
Author

I just pushed the commits, do you see them now?

@cswiercz
Copy link
Member

Yep! I do now. I see the change in your commit list. Thanks to both of you for being the guinea pigs. I'll alert the class of the fix.

@tgilbrough
Copy link
Author

No problem! Thanks for applying the fix so quick!

@cswiercz
Copy link
Member

A link to the course announcement: https://canvas.uw.edu/courses/1038251/discussion_topics/3319238

@cswiercz
Copy link
Member

@tgilbrough I think I'll find a way to give you some extra credit points for pointing out this bug...

@tgilbrough
Copy link
Author

Oh wow, thank you! One other small question, when writing tests using the mat_vecfunction in wrappers.py, should we expect it to return a one dimension long array, or a Nx1 array structured as a vector? Thanks!

@gadamico
Copy link

Is there supposed to be a merge conflict that we have to fix before pulling? (That's what I'm getting.)

@gadamico
Copy link

OK, I think I figured it out; I was in the wrong directory. I now believe I've successfully pulled--and pushed.

@cswiercz
Copy link
Member

@tgilbrough A standard 1d Numpy array, just like the input and what you would send to numpy.dot.

@tgilbrough
Copy link
Author

When I am testing a 3x2 matrix times a 2x1 matrix, A.dot(x) returns a vector with shape (3, 1), which is printed as a column vector. When using the method implemented as part of linalg.c, the output array is of shape (3, ), printed as a row vector. Both have the same exact values just different shapes. When I change the line in mat_vec function within wrappers.py from:
out = numpy.ascontiguousarray(numpy.zeros(M, dtype=numpy.double))
to
out = numpy.ascontiguousarray(numpy.zeros([M, 1], dtype=numpy.double))
The function works perfectly. Is my implementation incorrect, or should I consider the column and row vectors as the same? Thanks! Sorry for all the questions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants