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

Ivan khromov #1

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Ivan khromov #1

wants to merge 8 commits into from

Conversation

vano105
Copy link
Collaborator

@vano105 vano105 commented Jul 30, 2024

Added four matrix multiplication variants from the tutorial (https://cnugteren.github.io/tutorial/pages/page1.html). The first is naive, the second is related to local memory, the third is about enhancing performance with a single thread, and the fourth uses vector types. A Jupyter notebook has been added for automating the execution and adjustment of vortex parameters, as well as gathering launch statistics in a data frame.

@vano105 vano105 requested review from gsvgit and vkutuev July 30, 2024 13:43
Copy link

@vkutuev vkutuev left a comment

Choose a reason for hiding this comment

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

Also, you must change commit authors!!!

Comment on lines +26 to +31
" cores: int = 1\n",
" threads: int = 1\n",
"# running parameters \n",
"@dataclass\n",
"class run:\n",
Copy link

Choose a reason for hiding this comment

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

Suggested change
" cores: int = 1\n",
" threads: int = 1\n",
"# running parameters \n",
"@dataclass\n",
"class run:\n",
" cores: int = 1\n",
" threads: int = 1\n",
"\n",
"# running parameters \n",
"@dataclass\n",
"class run:\n",

"metadata": {},
"outputs": [],
"source": [
"path_to_vortex = \"/home/jblab/ivan_khromov/release/vortex\"\n",
Copy link

Choose a reason for hiding this comment

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

Do not use fixed absolute path! It must be calculated from relative.

Comment on lines 118 to 120
" if \"FAILED\" in line: \n",
" error_message = error_verification(run_params, line[line.find(\"FAILED! - \"):])\n",
" if \"Error\" in line:\n",
Copy link

Choose a reason for hiding this comment

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

It is better to check subprocess return code

Comment on lines 91 to 92
" with open(f\"{path_to_vortex}/tests/opencl/{kernel_name}/main.cc\", 'a') as main:\n",
" main.write('')"
Copy link

Choose a reason for hiding this comment

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

Maybe use Path.touch

"def create_common_h (params: dict, kernel_name: str):\n",
" file_name = f\"{path_to_vortex}/tests/opencl/{kernel_name}/common.h\"\n",
" with open(file_name, 'w') as file:\n",
" text = \"#ifndef COMMON_H\\n\" + \"#define COMMON_H\\n\" + \"\\n\" \n",
Copy link

Choose a reason for hiding this comment

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

Suggested change
" text = \"#ifndef COMMON_H\\n\" + \"#define COMMON_H\\n\" + \"\\n\" \n",
" file.write(\"#ifndef COMMON_H\\n\" + \"#define COMMON_H\\n\" + \"\\n\") \n",

Copy link

Choose a reason for hiding this comment

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

Same issue wit other lines

@vano105 vano105 force-pushed the ivan_khromov branch 2 times, most recently from b62ee90 to d5a9e5c Compare July 31, 2024 14:11
@vano105 vano105 requested a review from vkutuev July 31, 2024 14:13
@vano105 vano105 force-pushed the ivan_khromov branch 3 times, most recently from e134ad1 to 8520104 Compare July 31, 2024 17:48
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