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

Improve config handling in cpp and python #644

Open
wants to merge 17 commits into
base: branch-25.04
Choose a base branch
from

Conversation

kingcrimsontianyu
Copy link
Contributor

@kingcrimsontianyu kingcrimsontianyu commented Feb 24, 2025

Closes #642
Closes #526

This PR allows the properties in kvikio.defaults and kvikio.cufile_driver to be set in a friendly Pythonic way (C++ interface has also been changed). The expression below can appear standalone which sets the properties globally, or in a with statement which sets the properties via a context manager.

  • Set one or more properties
kvikio.defaults.set({"prop1": value1, "prop2": value2}):
kvikio.cufile_driver.properties.set({"prop1": value1, "prop2": value2})
  • Set a single property
kvikio.defaults.set("prop", value)
kvikio.cufile_driver.properties.set("prop", value)

@kingcrimsontianyu kingcrimsontianyu added breaking Introduces a breaking change improvement Improves an existing functionality python Affects the Python API of KvikIO c++ Affects the C++ API of KvikIO labels Feb 24, 2025
@kingcrimsontianyu kingcrimsontianyu self-assigned this Feb 24, 2025
Copy link

copy-pr-bot bot commented Feb 24, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

1 similar comment
@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

@kingcrimsontianyu kingcrimsontianyu marked this pull request as ready for review February 24, 2025 22:52
@kingcrimsontianyu kingcrimsontianyu requested review from a team as code owners February 24, 2025 22:52
@kingcrimsontianyu
Copy link
Contributor Author

Forgot about cuFile's DriverProperties. Sending this PR to draft and working on that.

@kingcrimsontianyu kingcrimsontianyu marked this pull request as draft February 24, 2025 22:59
@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

2 similar comments
@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

@kingcrimsontianyu
Copy link
Contributor Author

/ok to test

@kingcrimsontianyu kingcrimsontianyu marked this pull request as ready for review February 26, 2025 16:11
# TODO: Wrap nicely, maybe as a dataclass?
# <https://github.com/rapidsai/kvikio/issues/526>
DriverProperties = cufile_driver.DriverProperties
properties = cufile_driver.DriverProperties()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having an instance of DriverProperties() would suffice. So now the settable properties in cufile driver can be set in the following ways:

kvikio.cuda_driver.properties.<prop> = <value>
kvikio.cuda_driver.set("<prop>", <value>) # Can also be used in a "with" statement
kvikio.cuda_driver.set({"<prop>": <value>}) # Can also be used in a "with" statement

Let me know if this looks good enough to you @madsbk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Introduces a breaking change c++ Affects the C++ API of KvikIO improvement Improves an existing functionality python Affects the Python API of KvikIO
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusion around setting config values Pythonic bindings to cuFIle's driver properties
3 participants