-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fixes for GPU Interface #18
base: master
Are you sure you want to change the base?
Conversation
@TomTheBear Maybe you can review the PR? Thanks! |
@@ -95,6 +95,8 @@ def get_hierarchy(): | |||
|
|||
try: | |||
LIKWID_PREFIX, LIKWID_LIBPATH, LIKWID_LIB, LIKWID_INCPATH = get_hierarchy() | |||
# TODO | |||
major, minor, release = (5, 3, 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded version is not future-proof. The major and minor number is present in likwid.h
after installation, so maybe we read it from there.
Remark: There is no version 5.3.2
@@ -105,6 +107,7 @@ def get_hierarchy(): | |||
include_dirs=[LIKWID_INCPATH], | |||
libraries=[LIKWID_LIB], | |||
library_dirs=[LIKWID_LIBPATH], | |||
define_macros=[("LIKWID_MAJOR", major), ("LIKWID_MINOR", minor), (("LIKWID_RELEASE", release))], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? The variables major
, minor
and release
are defined in a try
block, so they are probably not in scope anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There exists a function likwidversion
that returns the version at runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the MACROS are not defined. It seems like there's a differently named MACRO in likwid.h, which comprises a string of the full version, .e.g., "5.3.2". So it seems like the function is broken when not defining the macros here
@@ -2683,7 +2686,7 @@ static PyMethodDef LikwidMethods[] = { | |||
{"nvgetnameofgroup", likwid_nvmon_getNameOfGroup, METH_VARARGS, "Return the name of a Nvmon group."}, | |||
{"nvgetshortinfoofgroup", likwid_nvmon_getShortInfoOfGroup, METH_VARARGS, "Return the short description of a Nvmon group."}, | |||
{"nvgetlonginfoofgroup", likwid_nvmon_getLongInfoOfGroup, METH_VARARGS, "Return the long description of a Nvmon group."}, | |||
{"nvgeteventsofgpu", likwid_nvmon_getEventsOfGpu, METH_VARARGS, "Get the events of a gpu."} | |||
// {"nvgeteventsofgpu", likwid_nvmon_getEventsOfGpu, METH_VARARGS, "Get the events of a gpu."}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this function not exported anymore and completely commented out? Are there any undefined symbols?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, calls to nvmon_returnEvents* etc. are undefined
Hello,
I discovered minor bugs in the GPU API that caused pylikwid to fail when building/importing:
Please feel free to provide more sophisticated solutions to my temporary fixes.