Algebraic-Programming / ALP

Home of ALP/GraphBLAS and ALP/Pregel, featuring shared- and distributed-memory auto-parallelisation of linear algebraic and vertex-centric programs. Soon with more to come!
Apache License 2.0
25 stars 4 forks source link

Issue #261: adding missing dense descriptor and delete unnecessary se… #288

Closed aristeidis-mastoras closed 7 months ago

aristeidis-mastoras commented 8 months ago

Closes issue #261 .

Closes issue #305 .

anyzelman commented 7 months ago

Note: I also added the todo of removing the invert_mask in the PR algorithm. If no objection I could do that quickly around now?

anyzelman commented 7 months ago

Note: I also added the todo of removing the invert_mask in the PR algorithm. If no objection I could do that quickly around now?

This is done. I did run into a new bug in nonblocking, I opened issue #305 for it.

anyzelman commented 7 months ago

@aristeidis-mastoras @byjtew -- this one is now conflicted but was the next on my list. I'll try resolve the conflicts now

anyzelman commented 7 months ago

Rebase complete, smoke tests with LPF OK. Unit tests with LPF and GitHub CI running overnight

anyzelman commented 7 months ago

(If anyone of @aristeidis-mastoras @byjtew wants to review how I handled the conflicts lemme know before tomorrow -- if no message by e.g. noon, I'll proceed with review/merge:)

anyzelman commented 7 months ago

Crossref #305 which this MR should also fix (TBC)

anyzelman commented 7 months ago

Review OK but found one issue in the bsp1d backend. Re-running smoke & unit tests with LPF.

anyzelman commented 7 months ago

All tests OK, will merge

anyzelman commented 7 months ago

Concept release notes:

Prior to this MR, the PageRank implementation does not pass the dense descriptor when it could, while it includes the use of the expensive invert_mask in its performance-critical loop. Mask inversion is expensive since it requires iterating over the full vector range, while, in this case, the inverted mask is constant. Hence, this MR introduces the following changes:

This improvement does come at the cost of one additional work-space vector that is now required for the PageRank implementation.

In implementing these fixes, the following feature has been added:

The following bugs were encountered and fixed with this MR:

anyzelman commented 7 months ago

Found one more minor issue in nonblocking, now fixed -- re-running unit tests excluding the LPF ones.