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

Single processor implementation for linalg.solve #568

Merged

Conversation

magnatelee
Copy link
Contributor

This extends #518 with some extra changes to make the code fully functional.

@magnatelee magnatelee added the category:new-feature PR introduces a new feature and will be classified as such in release notes label Sep 1, 2022
cunumeric/linalg/linalg.py Outdated Show resolved Hide resolved
cunumeric/linalg/linalg.py Outdated Show resolved Hide resolved
cunumeric/linalg/linalg.py Show resolved Hide resolved
examples/solve.py Outdated Show resolved Hide resolved
@magnatelee magnatelee mentioned this pull request Sep 1, 2022
@magnatelee
Copy link
Contributor Author

@rohany I added an optional output parameter to the internal API as I don't want to deviate from the NumPy API: https://github.com/nv-legate/cunumeric/pull/568/files#diff-809207875808b833ab90102d3ef7268ccff656c308004f73a82822903bf2b434R635 note that this function bypasses all argument checks in the user-facing version, so use it at your own risk.

@bryevdv
Copy link
Contributor

bryevdv commented Sep 1, 2022

LGTM

#endif

size_t a_strides[2];
VAL* a = a_array.read_write_accessor<VAL, 2>(a_shape).ptr(a_shape, a_strides);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a debugging check here, that

a_strides[0] == m && a_strides[1] == 1

and similarly for b_strides below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@magnatelee magnatelee Sep 9, 2022

Choose a reason for hiding this comment

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

Note that the assertion should be a_strides[0] == 1 && a_strides[1] == m, as this task assumes the instances to have the Fortran order.

Copy link
Contributor

@manopapad manopapad left a comment

Choose a reason for hiding this comment

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

One more debugging check suggested, otherwise looks good

@magnatelee magnatelee merged commit e5fc3cf into nv-legate:branch-22.10 Sep 12, 2022
@magnatelee magnatelee deleted the almogsegal-add-cunumeric-solve branch September 12, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-feature PR introduces a new feature and will be classified as such in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants