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

[SW] Add kernels: gemv, spmv, conjugate gradient #175

Closed
wants to merge 19 commits into from

Conversation

husterZC
Copy link
Contributor

Description of PR that completes issue here...

Added

  • spmv
  • gemv
  • conjugate gradient

@mp-17
Copy link
Collaborator

mp-17 commented Nov 11, 2022

Thanks a lot @husterZC, I will check it asap!

@husterZC
Copy link
Contributor Author

Hi Matteo, one thing to notice:

In the conjugate_gradient/shared_kernels. these are all soft link files, but I don't know why we I push them, they loss the link. Could you please fix them yourself, either make a hard copy or make a link file. And for some workloads using other kernels, do you suggest to make a link or a hard copy?

@mp-17
Copy link
Collaborator

mp-17 commented Nov 13, 2022

I would prefer symlinks to avoid duplicated code. I created a PR to your main: https://github.com/husterZC/ara/pull/1

[SW] Fix conjugate_gradient/shared_kernel simlinks
Copy link
Collaborator

@mp-17 mp-17 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the contribution!
Can you please pass everything through clang-format and adjust the vertical alignment in apps/common/default_args.mk?
Then, if the CI checks pass, LGTM!

apps/common/default_args.mk Outdated Show resolved Hide resolved
apps/conjugate_gradient/main.c Outdated Show resolved Hide resolved
@mp-17
Copy link
Collaborator

mp-17 commented Nov 22, 2022

@husterZC Ping me if you need support! :-)

@husterZC
Copy link
Contributor Author

To be honest, I'm busy on other stuff. I plan to fix this last in this week

@husterZC
Copy link
Contributor Author

Hey Matteo,

I've added the dependence of sklearn python package for conjugate gradient kernel.
We need this one to generate the symmetric and positive-defined (SPD) dense/sparse matrix for conjugate gradient
If you find it is a bit troublesome to add new dependency, there are other solutions:

  1. I can bring a pre-build SPD matrix and delete the function to generate SPD matrix at given size and sparsity. Well that would be very boring, and not very meaningful.
  2. I can look into how to generate SPD matrix at given size and sparsity, drive myself to be a mathematician and take somedays of working.

@mp-17
Copy link
Collaborator

mp-17 commented Nov 30, 2022

@mp-17 mp-17 changed the title add kernels: gemv, spmv, conjugate gradient [SW] Add kernels: gemv, spmv, conjugate gradient Nov 30, 2022
@husterZC
Copy link
Contributor Author

husterZC commented Dec 2, 2022

Hi Matteo, I wonder how to add the python package "sklearn" in the CI environment, otherwise it will always sticking here ;)

@mp-17
Copy link
Collaborator

mp-17 commented Dec 2, 2023

I will check everything and merge here -> #267
Thanks again Chi!

@mp-17 mp-17 closed this Dec 2, 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.

2 participants