fix experiment import bug and add it cases: experiment import #2878
Conversation
src/sdk/pynni/nni/msg_dispatcher.py
Outdated
| @@ -114,7 +114,7 @@ def handle_import_data(self, data): | |||
| data: a list of dictionaries, each of which has at least two keys, 'parameter' and 'value' | |||
| """ | |||
| for entry in data: | |||
| entry['value'] = json_tricks.loads(entry['value']) | |||
| entry['value'] = json_tricks.loads(str(entry['value'])) | |||
There was a problem hiding this comment.
what is the type of entry['value']? a dict/list object?
There was a problem hiding this comment.
In this simple case like [{"parameter": {...}, "value": 0.03}], it is <class 'float'>
There was a problem hiding this comment.
if it is float, why we invoke json_tricks.loads?
There was a problem hiding this comment.
maybe in some complex scene, user will set value as other types? like numpy array? This should invoke json_tricks.loads to converted back.
There was a problem hiding this comment.
i think entry['value'] should be a string, otherwise, there is a bug somewhere else.
| @@ -0,0 +1,4 @@ | |||
| [ | |||
| {"parameter": {"C": 0.15940134774738896, "kernel": "sigmoid", "degree": 3, "gamma": 0.07295826917955316, "coef0 ": 0.0978204758732429}, "value": 0.6}, | |||
There was a problem hiding this comment.
Because in search_space.json is "coef0 ", should I modify them all?
test/config/integration_tests.yml
Outdated
| configFile: test/config/nnictl_experiment/sklearn-classification.yml | ||
| launchCommand: nnictl resume $imoprtExpId | ||
| stopCommand: nnictl experiment export --filename config/nnictl_experiment/test_export.json --type json --intermediate | ||
| onExitCommand: nnictl stop |
There was a problem hiding this comment.
This is not the proper way to use stopCommand and onExitCommand.
Here the result of nnictl experiment export should be checked, you can use a validator to run export command and validate the results.
There was a problem hiding this comment.
I will try to modify them and add a validator.
…rt IT for duplicate
| import_data_file_path = kwargs.get('import_data_file_path') | ||
| proc = subprocess.run(['nnictl', 'experiment', 'import', exp_id, '-f', import_data_file_path]) | ||
| assert proc.returncode == 0, \ | ||
| '`nnictl experiment import {0} -f {1}` failed with code {2}'.format(exp_id, import_data_file_path, proc.returncode) |
There was a problem hiding this comment.
Any other means to verify the data is actually imported successfully besides checking the return code?
There was a problem hiding this comment.
I'll view the code again to find if there is a way to check the result.
There was a problem hiding this comment.
In order to check the imported data, add an api api/v1/nni/experiment/imported-data to get the imported data. I don't know if it is appropriate to do so. Maybe users also have demand to view the data they have imported?
There was a problem hiding this comment.
It is OK to call api/v1/nni/experiment/imported-data to verify
|
@SparkSnail Please help review |
In docs, experiment import data format example below,
valuecan be set as float, butjson_tricks.loads()only accept string, so fix it.Add IT cases about experiment import and export.