Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]Fix MatrixRef::lbegin/lend #428

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

Conversation

devreal
Copy link
Member

@devreal devreal commented Aug 3, 2017

This PR fixes two issues with MatrixRef lbegin/lend:

  1. Fixed compiler error
  2. Fixed lend() in LocalMatrixRef (and thus MatrixRef) to not return a nullptr but a pointer past the end of the range (GlobViewIter::local() returns nullptr if pointing beyond the current unit)

@devreal devreal added this to the dash-0.3.0 milestone Aug 3, 2017
@devreal devreal self-assigned this Aug 3, 2017
@devreal devreal changed the title Fix MatrixRef::lbegin/lend [WIP]Fix MatrixRef::lbegin/lend Aug 3, 2017
@devreal
Copy link
Member Author

devreal commented Aug 3, 2017

The offsets are still broken and I cannot seem to figure out which offsets to use. Will look at it again tomorrow... Any help is appreciated, though.

@devreal
Copy link
Member Author

devreal commented Aug 4, 2017

@fuchsto I'm giving up here as it is not clear to me anymore which are global and local block coordinates and indexes. Also, I suspect that there are fixes lingering around in your branches. I'd appreciate a timely merge or a quick fix as the broken lbegin() on blocks is a road block for me right now.

@fuchsto
Copy link
Member

fuchsto commented Sep 20, 2017

@devreal Sorry, I don't get it: what's the original issue? Is the use case covered in the unit test you added?
I found a bug related to #418 and will fix it in a separate PR, not sure if it helps your use case.

@devreal
Copy link
Member Author

devreal commented Oct 1, 2017

Sorry, I don't get it: what's the original issue?

This PR was my attempt to fix Matrix::lbegin a month after I reported the issue in #418 but I got lost in the usage of CartesianIndexSpace (does it hold local or global indexes? pretty confusing for me) My original use case should be covered by the test case I added here.

@devreal devreal modified the milestones: dash-0.3.0, dash-0.4.0 Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants