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.

Sign up for GitHub

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

Jump to bottom

Program.compile() options parameter is not handled properly #221

Open
ksimpson-work opened this issue Nov 6, 2024 · 2 comments · May be fixed by #237
Open

Program.compile() options parameter is not handled properly #221

ksimpson-work opened this issue Nov 6, 2024 · 2 comments · May be fixed by #237
Assignees
@ksimpson-work
Labels
cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P1 Medium priority - Should do

Comments

@ksimpson-work
Copy link
Contributor

ksimpson-work commented Nov 6, 2024 •
edited
Loading

compile() requires the options parameter to be an iterable which is inconsistent with the use of @ dataclass to support options to the other methods (device.create_stream etc.) This is not a bug, but rather an possible ux enhancement.

calling : Program(empty_entrypoint_kernel, "c++").compile("ltoir", ("-dlto"))

cuda.core.experimental._utils.NVRTCError: 5: NVRTC_ERROR_INVALID_OPTION, compilation log:
nvrtc: error: unrecognized option - found

("dlto",) would succeed.
@ksimpson-work ksimpson-work added bug Something isn't working cuda.core Everything related to the cuda.core module P0 High priority - Must do! labels Nov 6, 2024
@ksimpson-work ksimpson-work self-assigned this Nov 6, 2024
@ksimpson-work ksimpson-work added P1 Medium priority - Should do enhancement Any code-related improvements and removed bug Something isn't working P0 High priority - Must do! labels Nov 7, 2024
@leofang
Copy link
Member

leofang commented Nov 11, 2024 •
edited
Loading

Let me piggyback on this issue to record our offline discussion: options should be moved from compile() to the Program() constructor, for better consistency with the Linker (#155) and reflecting the intention that options is a part of the Program/Linker definition.

@ksimpson-work
Copy link
Contributor Author

#237

@leofang leofang linked a pull request Nov 14, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ksimpson-work ksimpson-work

Labels
cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P1 Medium priority - Should do
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add the options data class to program NVIDIA/cuda-python
2 participants
@leofang @ksimpson-work