envd icon indicating copy to clipboard operation
envd copied to clipboard

feat: Better envdlib syntax support

Open aseaday opened this issue 2 years ago • 7 comments

Description

When I am working on writing envdlib for supporting tensorRT, I found some points we need for better use of envdlib:

  • Branch Judgment: we need judge wether current condition like os version and cuda version supported by tensorRT
  • Print: sometimes we should print some information for user's to use such as the location of sdk or agreement default to accept.
  • Context Varibles: we need context varibales such as os version to decide how to install

We still need to some work to implement composable image building. We still use the process-oriented pattern to acheive my goal. Maybe it is not a good idea in the future. https://nixos.org/ can give us some guides.


Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

aseaday avatar Nov 01 '22 08:11 aseaday

/cc @kemingy

gaocegege avatar Nov 01 '22 08:11 gaocegege

  • Related to https://github.com/tensorchord/envd/pull/972
* Branch Judgment: we need judge wether current condition like os version and cuda version supported by tensorRT

* Print: sometimes we should print some information for user's to use such as the location of sdk or agreement default to accept.

I'm wondering if the functions can return some kind of structure. But surely that will break the current process-oriented pattern.

* Context Varibles: we need context varibales such as os version to decide how to install

To pass the information to users, I think we need to expose the Graph (or something equivalent).

kemingy avatar Nov 01 '22 10:11 kemingy

Generally, this means function can share information across different function.

Solution 1: Use global variable-like mechanism For example, using provide and inject (idea from vue3)

def cuda(version="11.6.2"):
    provide("cuda/version", "11.6.2")

def torch():
    # inject(key, default_value)
    cuda_version = inject("cuda/version", "10.2") 
    install.python_package(["torch-cu{}".format(cuda_version)])

Then user can directly call envdlib.torch() to install the version related to cuda version. Note the execution needs to be ordered, user have to execute install.cuda before envdlib.torch

Solution 2: Expose the underlying build graph

def torch():
    # _get_build_graph convert build graph into python dict
    info_dict = _get_build_graph()
    version = info_dict["cuda"]

VoVAllen avatar Nov 09 '22 09:11 VoVAllen

Solution 1: Use global variable-like mechanism For example, using provide and inject (idea from vue3)

def cuda(version="11.6.2"):
    provide("cuda/version", "11.6.2")

def torch():
    # inject(key, default_value)
    cuda_version = inject("cuda/version", "10.2") 
    install.python_package(["torch-cu{}".format(cuda_version)])

I feel defining a global variable could be easier.

cuda_version = "11.6.2"

def build():
    install.cuda(cuda_version)
    envdlib.tensorrt(cuda=cuda_version)

We should document the if-else, for, string, print (debug maybe), etc.

Ref: https://github.com/bazelbuild/starlark/blob/master/spec.md

kemingy avatar Nov 09 '22 09:11 kemingy

@kemingy Arbitrary global variable doesn't seem good. At least a global dictionary needed I think. such as we have a global variable such as global_info and user can do anything they like on it

VoVAllen avatar Nov 09 '22 10:11 VoVAllen

@kemingy Arbitrary global variable doesn't seem good. At least a global dictionary needed I think. such as we have a global variable such as global_info and user can do anything they like on it

I mean users can define their own global variables to use.

It seems users don't know what starlark can do. And how to combine starlark built-in features with envd functions.

kemingy avatar Nov 09 '22 10:11 kemingy

I think we'd better give a spec for envdlib's contributor such as:

  • how to define/get metadata need or provide
  • print or debug some information

But for normal users, we could just let them know they can treat starlark like python and builtin functions list.

aseaday avatar Nov 09 '22 10:11 aseaday