Skip to content

SpMV#49

Merged
gsvgit merged 8 commits intoSparseLinearAlgebra:net5from
kirillgarbar:spmv
Nov 14, 2022
Merged

SpMV#49
gsvgit merged 8 commits intoSparseLinearAlgebra:net5from
kirillgarbar:spmv

Conversation

@kirillgarbar
Copy link
Copy Markdown
Contributor

Proposed Changes

SpMV operation added

Types of changes

What types of changes does your code introduce to GraphBLAS-sharp?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • Build and tests pass locally
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

@gsvgit gsvgit requested review from artemgl and gsvgit November 7, 2022 08:59
.GetDeviceInfo(clContext.ClDevice.Device, OpenCL.Net.DeviceInfo.LocalMemSize, error)
.CastTo<int>()

let localArraySize1 = workGroupSize + 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be reused in other algorithms to calculate the optimal size of the local array?

let rowEnd =
min (localPtr.[lid + 1] - blockLowerBound) workPerIteration

for jj in rowStart .. rowEnd - 1 do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason to use such a strange name jj?

|> correctnessGenericTest 0uy (+) (*) byteAdd (=) q
|> testPropertyWithConfig config (getCorrectnessTestName "byte") ]

let tests =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like code duplication (look here or to other tests)

localMemoryLeft / optionTypeClSizeInBytes
|> Utils.floorToMultiple workGroupSize

let kernel1 =
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite friendly names

min (localPtr.[lid + 1] - blockLowerBound) workPerIteration

for jj in rowStart .. rowEnd - 1 do
match (%add) sum localValues.[jj] with
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This match is equivalent to sum <- (%add) sum localValues.[jj]

//Loading values to the local memory
for block in 0 .. numberOfBlocksFitting - 1 do
if index < workEnd then
localValues.[lid + block * threadsPerBlock] <- intermediateArray.[index]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like if it's the last work group, indices are out of bounds sometimes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the last work group threadsPerBlock is computed as numberOfRows - gid + lid, so workEnd will be equal to the last value of row pointers array and indices will not be out of bounds

@gsvgit gsvgit requested a review from artemgl November 14, 2022 10:16
@gsvgit gsvgit merged commit 4422318 into SparseLinearAlgebra:net5 Nov 14, 2022
@kirillgarbar kirillgarbar deleted the spmv branch March 24, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants