GPU Lower Triangular Inverse

[SteveBronder]

Summary

Adds the kernels and functions for performing inversions of lower triangular matrices on the GPU.

At the stan math user level, this PR adds the function lower_triangular_inverse(A) that accepts a matrix_gpu. Internally, we add 3 kernels which break the inversion into three separate steps following the steps described on page 2 of the Stan OpenCL paper. step1 performs a matrix inversion with no blocking on the top lower triangular while step2 and step3 Calculates the intermediate and final products needed in the parallel blocked version of the inverse.

Tests

Tests are available in lower_triangular_inverse_test.cpp

1. inverse_gpu_exception

• Checks for an std::invalid_argument on a non-square matrix_gpu

2. inverse_gpu_small

• Checks whether the CPU and GPU version of the matrix inverse have near the same level of precision.

3. inverse_gpu_big

• Checks whether the CPU and GPU version of the matrix inverse have near the same level of precision.

Checklist

• Math issue Add GPU Inverse of a lower triangular matrix #1028

• Copyright holder: (fill in copyright holder information)

By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

• Rok Češnovar and Erik Štrumbelj (Faculty of Computer and Information Science, University of Ljubljana)

• Steve Bronder

• the basic tests are passing

  • unit tests pass (to run, use: ./runTests.py test/unit)
  • header checks pass, (make test-headers)
  • docs build, (make doxygen)
  • code passes the built in C++ standards checks (make cpplint)

• the code is written in idiomatic C++ and changes are documented in the doxygen

• the new changes are tested

[SteveBronder]

I added a few things from the paper and changed the names. I think these names are pretty fine. Rok if you have disagreements on those then I'm very open to changing them, though I think they are an okay mix of shorthand and 'getting the point across'. We should make a wiki page out of the google doc and link to it in each kernel.

[SteveBronder]

Either Jenkins is down or I goofed up p bad

[rok-cesnovar]

I am getting a 404 error on the Jenkins Details link, so I guess its down.

[seantalts]

I installed a security update and that has made a lot of the logged out links not work for some reason. Can you try logging in on the home page and then clicking the link?

[rok-cesnovar]

@seantalts It works! Thanks

[bob-carpenter]

The math library has been inconsistent in how verbose the names are, but we've been pretty consistent with diagonal -> diag inverse -> inv negative -> neg rectangular -> rect triangular -> tri which would yield the much less unwieldy diag_inv inv_lower_tri_multiply neg_rect_lower_tri_multiply

[SteveBronder]

I like those and agree, changing now. Also I'm cleaning up the developer docs and placing it in the stancon2018 GPU repo as a pdf. I think including a link to it in the code docs will be the easiest way to give people who want a deeper understanding of the implementation more background. Is that alright with everyone?

[seantalts]

I think we can and should incorporate that doc into the code, I just haven't had time to work on it yet. Hopefully tomorrow. I think we can doc the high level overview under stan/math/gpu/lower_tri_inverse.hpp and put the rest of the step specific doc in with the steps - there are even parts of the code copied into the doc, so just doc'ing the code where it lies should be more efficient. Plus doc that lives closer to the code is less likely to become out-of-date.

[rok-cesnovar]

I will take a crack at this. Should I link to the images or just ignore them?

[SteveBronder]

Oh nice I agree with that. I spent some time tonight cleaning up the google doc a bit, if you have time tmrw to work on this that would be rad!

[seantalts]

Good question - I think they're really helpful, and it seems easier to link than to include them in the repo. I'd throw them up on the Stan Math wiki somewhere and link to them from there.

…

On Mon, Oct 15, 2018 at 11:58 PM Rok Češnovar ***@***.***> wrote: I will take a crack at this. Should I link to the images or just ignore them? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1030 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAxJ7FGYQQKJsQ6-PyTZspk7DZ32xuiIks5ulVlMgaJpZM4Wmezj> .

[rok-cesnovar]

I have added everything on the wiki, together with the images that will be linked in the code docs. Link: https://github.com/stan-dev/math/wiki/GPU-Kernels

[seantalts]

Sorry, I meant that I wanted to bring the doc into the code as doc strings. So I'm going to try to move all of the text into the code files that already exist and hopefully generate good Doxygen from that. And I'll have to link to images on the wiki. Sorry I've been so busy - back to back conferences and just gave a talk yesterday, but I'll try to set aside some time tonight.

[SteveBronder]

Alright so I changed the names to what Bob suggested and added the images and a lot of comments to the kernel code. @seantalts lmk how you feel about this

[seantalts]

Looks good! I went through and polished the doc a little; I think I probably gave you guys the impression I was looking for more detailed doc than I needed. I was hoping you guys could look at this diff and 1) make sure I didn't mess anything up and 2) get a feel for the level of doc that we're looking for - which stuff I deleted, which stuff I added from the wiki. @SteveBronder already did a pass adding in basically all the relevant stuff from the wiki, so that was great. In general we'd prefer doc to live with the code when possible, we usually don't ask people to doc code that is doing fairly standard stuff (but that is obviously a relative question especially when introducing a new framework to an existing project), and if you can give something a descriptive name and delete a comment that's usually a win. I did this in one place (seemed like 'factor' was less helpful as a name than the comment above it that it was the diagonal element) but I need to double check I did that right.

Here's the diff from my two commits:
https://github.com/stan-dev/math/pull/1030/files/34bc01405cf8e0997ca52d2029255bdd6e84b871..416b0970a5becda77c6550d0ded2d3a371793b8e

[SteveBronder]

Looks good besides one little extra star I saw. Thanks!