gemini-code-assist[bot] commented on code in PR #334:
URL: https://github.com/apache/tvm-ffi/pull/334#discussion_r2612508404
##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -1071,43 +1078,48 @@ def load(
Parameters
----------
- name: str
+ name
The name of the tvm ffi module.
- cpp_files: Sequence[str] | str, optional
+ cpp_files
The C++ source files to compile. It can be a list of file paths or a
single file path. Both absolute and
relative paths are supported.
- cuda_files: Sequence[str] | str, optional
+ cuda_files
The CUDA source files to compile. It can be a list of file paths or a
single file path. Both absolute and
relative paths are supported.
- extra_cflags: Sequence[str], optional
+ extra_cflags
The extra compiler flags for C++ compilation.
The default flags are:
- On Linux/macOS: ['-std=c++17', '-fPIC', '-O2']
- On Windows: ['/std:c++17', '/MD', '/O2']
- extra_cuda_cflags: Sequence[str], optional
+ extra_cuda_cflags
The extra compiler flags for CUDA compilation.
The default flags are:
- ['-Xcompiler', '-fPIC', '-std=c++17', '-O2'] (Linux/macOS)
- ['-Xcompiler', '/std:c++17', '/O2'] (Windows)
- extra_ldflags: Sequence[str], optional
+ extra_ldflags
The extra linker flags.
The default flags are:
- On Linux/macOS: ['-shared', '-L<tvm_ffi_lib_path>', '-ltvm_ffi']
- On Windows: ['/DLL', '/LIBPATH:<tvm_ffi_lib_path>',
'<tvm_ffi_lib_name>.lib']
- extra_include_paths: Sequence[str], optional
+ extra_include_paths
The extra include paths for header files. Both absolute and relative
paths are supported.
- build_directory: str, optional
+ build_directory
The build directory. If not specified, a default tvm ffi cache
directory will be used. By default, the
cache directory is ``~/.cache/tvm-ffi``. You can also set the
``TVM_FFI_CACHE_DIR`` environment variable to
specify the cache directory.
+ keep_module_alive
+ Whether to keep the loaded module alive to prevent unloading. If True,
the module will be kept alive
+ until the program exits, unless manually unloaded via
`Module.close()`. If False, the module may be unloaded
+ when there are no more references to it.
Review Comment:
This docstring has the same issue as in `load_inline`, mentioning a
non-existent `Module.close()` method. Please ensure consistency and accuracy
across all related docstrings.
```suggestion
keep_module_alive
Whether to keep the loaded module alive to prevent unloading. If
True, the module is
kept alive for the program's duration. If False, the module may be
unloaded when
there are no more references to it.
```
##########
python/tvm_ffi/module.py:
##########
@@ -427,14 +427,18 @@ def system_lib(symbol_prefix: str = "") -> Module:
return _ffi_api.SystemLib(symbol_prefix)
-def load_module(path: str | PathLike) -> Module:
+def load_module(path: str | PathLike, keep_module_alive: bool = True) ->
Module:
"""Load module from file.
Parameters
----------
path
The path to the module file.
+ keep_module_alive
+ Whether to keep the module alive. If True, the module will be kept
alive
+ for the duration of the program unless libtvm_ffi.so is unloaded.
Review Comment:
For completeness and consistency with other `load` functions, it would be
helpful to also describe the behavior when `keep_module_alive` is `False`.
```suggestion
keep_module_alive
Whether to keep the module alive. If True, the module is kept alive
for the
program's duration. If False, it may be unloaded when no references
remain.
```
##########
python/tvm_ffi/cpp/extension.py:
##########
@@ -791,56 +792,60 @@ def load_inline(
Parameters
----------
- name: str
+ name
The name of the tvm ffi module.
- cpp_sources: Sequence[str] | str, optional
+ cpp_sources
The C++ source code. It can be a list of sources or a single source.
- cuda_sources: Sequence[str] | str, optional
+ cuda_sources
The CUDA source code. It can be a list of sources or a single source.
- functions: Mapping[str, str] | Sequence[str] | str, optional
+ functions
The functions in cpp_sources or cuda_source that will be exported to
the tvm ffi module. When a mapping is
given, the keys are the names of the exported functions, and the
values are docstrings for the functions
(use an empty string to skip documentation for specific functions).
When a sequence or a single string is given, they are
the functions needed to be exported, and the docstrings are set to
empty strings. A single function name can
also be given as a string. When cpp_sources is given, the functions
must be declared (not necessarily defined)
in the cpp_sources. When cpp_sources is not given, the functions must
be defined in the cuda_sources. If not
specified, no function will be exported.
- extra_cflags: Sequence[str], optional
+ extra_cflags
The extra compiler flags for C++ compilation.
The default flags are:
- On Linux/macOS: ['-std=c++17', '-fPIC', '-O2']
- On Windows: ['/std:c++17', '/O2']
- extra_cuda_cflags: Sequence[str], optional
+ extra_cuda_cflags
The extra compiler flags for CUDA compilation.
- extra_ldflags: Sequence[str], optional
+ extra_ldflags
The extra linker flags.
The default flags are:
- On Linux/macOS: ['-shared']
- On Windows: ['/DLL']
- extra_include_paths: Sequence[str], optional
+ extra_include_paths
The extra include paths.
- build_directory: str, optional
+ build_directory
The build directory. If not specified, a default tvm ffi cache
directory will be used. By default, the
cache directory is ``~/.cache/tvm-ffi``. You can also set the
``TVM_FFI_CACHE_DIR`` environment variable to
specify the cache directory.
- embed_cubin: Mapping[str, bytes], optional
+ embed_cubin
A mapping from CUBIN module names to CUBIN binary data. When provided,
the CUBIN data will be embedded
into the compiled shared library using objcopy, making it accessible
via the TVM_FFI_EMBED_CUBIN macro.
The keys should match the names used in TVM_FFI_EMBED_CUBIN calls in
the C++ source code.
+ keep_module_alive
+ Whether to keep the loaded module alive to prevent unloading. If True,
the module will be kept alive
+ until the program exits, unless manually unloaded via
`Module.close()`. If False, the module may be unloaded
+ when there are no more references to it.
Review Comment:
The docstring for `keep_module_alive` mentions `Module.close()`, but this
method does not seem to exist on the `Module` class, which could be misleading.
For consistency and clarity, consider using a more concise and accurate
description across all functions (`load_module`, `cpp.load_inline`, `cpp.load`).
```suggestion
keep_module_alive
Whether to keep the loaded module alive to prevent unloading. If
True, the module is
kept alive for the program's duration. If False, the module may be
unloaded when
there are no more references to it.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
