Fixes for a series of issues + Print GFLOPs

[jlgreathouse]

This patchset fixes issues: #146 and #161. It also reverts the problem raised in this discussion.

To fix #146, I've added another variable to the control structure that controls whether explicit zero values are read in from the matrix market reader. I've changed the default such that we do read the zero values (as discussed in #144). Calling '-z' on many of our test applications now disables reading of explicit zeroes. I have not changed our sample applications.

The fix for #161 is as discussed in the issue.

Finally, I've changed the benchmarking code for SpMdV slightly. It now, in addition to printing out the bandwidth in GiB/s, prints out the canonical GFLOPs calculation (where the number of FLOPs is 2 * NNZ).

The GFLOPs calculation is not entirely correct when alpha !=1 and beta !=0, but the calculation as written is used in almost every academic study of SpMdV. Solely presenting GiB/s makes it difficult for people to compare results with clSPARSE. We could make the calculation more complex by special-casing the calculation when alpha and beta are different, but this seems decent enough for now (IMO).

[kknox]

I took a look over this PR; I think it's an interesting decision to make the explicit zeroes setting a part of the control object. I initially assumed we would implement this as an extra flag to the io API. This implies that we want to 'remember' this setting. Did you mean this; is this a setting that we want to keep track of?

[jlgreathouse]

I'll admit that I hadn't put a lot of thought into it; I mostly added it to the control structure for the sake of expediency. It was easier to add it into the control structure than to trace all of the steps between reading argv and the call to read the file and then add in logic to pass the variable.

[kknox]

@kvaragan @jpola I would be interested in what you think, do we keep explicit zeroes as state in the control object or pass a bool through the reader API?

I think I see some problems keeping it as state; the user might toggle that value before they call a computation routine. I don't think our library would actually be able to 'rely' on that value for any form of optimization, because we would somehow have to treat it as some form of constant. Possibly, 'explicit zeros' is state more appropriately associated with the sparse data loaded clsparseCsrMatrix, and not really the control object used to control the loading.

[kvaragan]

dense2csr benchmarking fails. Test input: Dubcova1.mtx from small MTX, tried with both options -z & without -z. Assertion failed: coo->num_nonzeros > 0, file C:\libteam\code\github\clMathLibra ries\clSPARSE\src\library\transform/conversion-utils.hpp, line 363

[kvaragan]

I will hold this PR for the time-being.

[jpola]

Hi,

I understand @jlgreathouse approach. It is definitely easier to keep the explicit_zeros flag in the control object because it is passed to almost every function in the library. On the other side this flag does not have any influence on other functions in the library. It is related only to the I/O functions. Therefore in my opinion it should be an independent flag of the I/O function, It should not be related to control object.

[kvaragan]

I agree with @jpola, Just pass bool to the reader API.

[kknox]

@jlgreathouse I think we are going to wait on merging this PR until after we do the beta2 release. If it's possible for you to separate the fixes for #150 and #161 today, I think that's low risk enough but otherwise I don't mind pushing a patch release of v0.8.1 at a later date to fix these bugs.

[jlgreathouse]

Sure thing. I'll submit a PR that doesn't include the explicit zeroes work and make the requested changes for later review.

[kknox]

Btw, I just merged a PR from Kiran that fixed #161 by substituting 'enable' for 'require'

[jlgreathouse]

That PR doesn't fully fix the issue (the "require" syntax also exists in csrmv_adaptive.cl, and atomic_reduce.cl should still check the OpenCL version).

[kknox]

OK, I guess just make sure to rebase his changes in so that you don't have merge conflicts :beer:

[jlgreathouse]

Will do. I'll do a new PR in a bit.