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

[bug] Fix SparseMatrix's dtype; check for dtype in SparseSolver. #8071

Merged
merged 2 commits into from
May 31, 2023

Conversation

houkensjtu
Copy link
Contributor

@houkensjtu houkensjtu commented May 25, 2023

Issue: #8045

Brief Summary

SparseMatrix's dtype should be determined by the dtype of the SparseMatrixBuilder from which it's built. However, it's fixed to f32 in the current code base. This PR fix this by passing self.dtype instead of dtype to SparseMatrix() in the builder:

def build(self, dtype=f32, _format="CSR"):                                                                                                                                                                                          
        """Create a sparse matrix using the triplets"""                                                                                                                                                                                 
        taichi_arch = get_runtime().prog.config().arch                                                                                                                                                                                  
        if taichi_arch in [_ti_core.Arch.x64, _ti_core.Arch.arm64]:                                                                                                                                                                     
            sm = self.ptr.build()                                                                                                                                                                                                       
            return SparseMatrix(sm=sm, dtype=self.dtype)   # Previously it was dtype, which is f32 in the current context.

Also, SparseSolver should raise an exception to better notify the user if the dtype of the SparseMatrix is not consistent with the solver's dtype. This is implemented in the sparse_solver.py.

Additional comments

@netlify
Copy link

netlify bot commented May 25, 2023

Deploy Preview for docsite-preview ready!

Name Link
🔨 Latest commit 20c6ee9
🔍 Latest deploy log https://app.netlify.com/sites/docsite-preview/deploys/646f11140a4b690008b9e494
😎 Deploy Preview https://deploy-preview-8071--docsite-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@houkensjtu
Copy link
Contributor Author

Please review this PR when you have the time @FantasyVR . Thanks :)

@houkensjtu houkensjtu requested a review from FantasyVR May 26, 2023 03:18
Copy link
Collaborator

@FantasyVR FantasyVR left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

Copy link
Contributor

@neozhaoliang neozhaoliang left a comment

Choose a reason for hiding this comment

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

LGTM!

@feisuzhu feisuzhu merged commit e3cccb8 into taichi-dev:master May 31, 2023
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.

4 participants