Skip to content

Conversation

Damfilius
Copy link

…the user prior to campaign running

codmmented out dependencies method - wrong dependencies were specified - needs to be fixed
post-run-hook - appended extension to report file name for successfull loading by ncu_report
[3rd variable] Height of matrix b has its own variable
[benchmark parsing] parsing of output from matMul
@apaolillo
Copy link
Collaborator

thanks for the PR. Do you version code here taken from somewhere else? @Damfilius

@Damfilius
Copy link
Author

thanks for the PR. Do you version code here taken from somewhere else? @Damfilius

Yes. I committed some code from CUDA samples and the ngAP repo, although I should have included the README files mentioning where I got them from in the respective directories.

@apaolillo
Copy link
Collaborator

thanks for the PR. Do you version code here taken from somewhere else? @Damfilius

Yes. I committed some code from CUDA samples and the ngAP repo, although I should have included the README files mentioning where I got them from in the respective directories.

We usually don't do that. We prefer a readme that indicate how to download the source from a remote repository.
See for example: https://github.com/open-s4c/benchkit/blob/main/tutorials/leveldb-bench/README.md
Could you update your PR accordingly @Damfilius ?

@Damfilius
Copy link
Author

I should have removed all of the code taken from other repos.

@apaolillo
Copy link
Collaborator

@Damfilius dear Damjan. There appear to be conflicts between your branch and the main branch of benchkit. It looks like we did implement ncu in parallel. Could you rebase your branch to avoid conflicts? Your branch will be next in line for integration. You probably need to rename your "ncu" into "ncu2" or something.

@Damfilius
Copy link
Author

Dear Prof. Paolillo, when pulling from the main branch I did not get any conflicts, nonetheless I renamed my ncu file to ncu2 and updated any imports.

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