Skip to content

Conversation

@aleksamilisavljevic
Copy link
Contributor

Closes #268

@aleksamilisavljevic aleksamilisavljevic linked an issue Feb 21, 2024 that may be closed by this pull request
@aleksamilisavljevic aleksamilisavljevic changed the title 268 provide grbsetmatrix out matrix mask matrix in 268 Provide grb::set(Matrix out, Matrix mask, Matrix in) Feb 21, 2024
Copy link
Collaborator

@byjtew byjtew left a comment

Choose a reason for hiding this comment

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

Nice job, as for the unit-test, you could rely on the matrix factories that we have.
e.g: L masked by I should return I
L being a Lower triangular matrix
I being an identity matrix

@anyzelman anyzelman added this to the v0.8 milestone Aug 17, 2025
@anyzelman anyzelman force-pushed the 268-provide-grbsetmatrix-out-matrix-mask-matrix-in branch from b8bf1eb to 3e33029 Compare August 17, 2025 14:50
@anyzelman
Copy link
Member

@aleksamilisavljevic @byjtew -- apologies, I thought this one was merged ages ago but found it still open just now. I've rebased it (and weirdly enough indeed a component from this MR was already in utils?) and will look at it more shortly: at the moment it builds but not sure if it passes unit tests, plus I see a spec for the missing primitive should still be provided). I'll take those remainder, hopefully small, steps over the coming days:)

@anyzelman
Copy link
Member

anyzelman commented Aug 23, 2025

It's back up with unit tests passing, still a few TODOs:

  • check if it makes sense to unit test non-void to void, no-casting vs. casting, and void-to-void. At present, the unit test goes for non-casting and non-void to non-void only
  • unit test should check passing empty masks
  • BSP1D and hybrid implementations are missing
  • reference_omp variant is not yet fully parallel

@anyzelman
Copy link
Member

Upgraded unit test -- running the full suite before continuing some other time

@anyzelman
Copy link
Member

anyzelman commented Aug 26, 2025

All tests pass at this point, by the way (test results from over the weekend and including for LPF). Only one TODO remaining, which is the parallelisation of this function in the reference_omp backend.

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.

Provide grb::set(Matrix out, Matrix mask, Matrix in)

4 participants