[SDK] Improve Validation for Objective Function

[andreyvelich]

/kind feature

Check this thread: #1951 (comment).
We need to improve validation for objective function in tune API.
For example, validate that user's imports are properly declared in the function.

---

Love this feature? Give it a 👍 We prioritize the features with the most 👍

[andreyvelich]

/area sdk

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tenzen-y]

/lifecycle frozen

[hnanacc]

Hello,

I was trying to work on this issue and wanted to know if one of the following solutions could be suitable for our use-case?

1. Use regex to extract imported names and compare with first-level symbols in function scope to find unimported names.

import inspect

def objective(params):
    import time
    def run_step():
        stt = time.perf_counter()
        acc = transformers.train()
        ent = time.perf_counter()
        return acc

    return run_step()

source = inspect.getsource(objective)

imported_names = get_imported_names(source)

# OUTPUT: 
# ['time']

other_tokens = get_nonimport_tokens(source)

# OUTPUT:
# ['run_step', 'ent', 'stt', 'time', 'transformers', 'acc']

unimported = compare_and_filter_defined(imported_names, other_tokens)

# OUTPUT:
# ['transformers']

NOTE: This approach could be tricky and may miss some edge cases. Further, this may also be hard to extend/maintain depending on the kind of validation we may want to do in the future.

2. Use a linter (ex. pylint) to lint the code string and check for the required errors.

import inspect
from astroid import AstroidBuilder
from pylint.lint import PyLinter

def objective(params):
    import time
    def run_step():
        stt = time.perf_counter()
        acc = transformers.train()
        ent = time.perf_counter()
        return acc

    return run_step()

ast_builder = AstroidBuilder(...)
ast_linter = PyLinter(...)

source = inspect.getsource(objective)

ast_repr = ast_builder.string_build(source)         # builds the abstract syntax tree (AST)
checked = ast_linter.check_astroid_module(ast_repr) # lints the AST, given checkers.

unimported = get_import_errors(ast_linter)          # see if any 'import-error E0401' messages exists.

# OUTPUT:
# ['transformers']

NOTE: For this approach, we need to add an additional dependency, which may already be present for linting, but in return we get correctness and simpler code which should be easier to maintain. We can also use the pylint cli with the required validations, as we are anyways creating a file for the objective function.

$ pylint $program_path/ephemeral_objective.py

Please let me know if any of the above approach could be implemented or if there is an alternative.

Thanks,
Harshal

[andreyvelich]

Thank you for your suggestions @hnanacc, it looks good.
Maybe we should try to explore if we have any ways to serialize function for Kubernetes container without asking using to make all import embedded under training function ?
I know that CloudPickle: https://github.com/cloudpipe/cloudpickle can help us to ship Python code over network, but maybe we miss something from inspect library.