uwhpsc-2016 / syllabus

Spring 2016 Course Syllabus and Information
15 stars 20 forks source link

Bug in wrappers.py #30

Open tgilbrough opened 8 years ago

tgilbrough commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

tgilbrough commented 8 years ago

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 commented 8 years ago

You misspelled "uwhpsc" in the URL. Use

$ git remote set-url upstream <newurl>

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

ckchmod commented 8 years ago

git upstream works for me

cswiercz commented 8 years ago

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

tgilbrough commented 8 years ago

Spelling things right makes quite the difference, thanks!

cswiercz commented 8 years ago

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

ckchmod commented 8 years ago

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

cswiercz commented 8 years ago

👍 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 commented 8 years ago

@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 commented 8 years ago

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

tgilbrough commented 8 years ago

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

cswiercz commented 8 years ago

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 commented 8 years ago

No problem! Thanks for applying the fix so quick!

cswiercz commented 8 years ago

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

cswiercz commented 8 years ago

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

tgilbrough commented 8 years ago

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 commented 8 years ago

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

gadamico commented 8 years ago

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

cswiercz commented 8 years ago

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

tgilbrough commented 8 years ago

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