Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Fix search space compatibility with JSON #4455

Merged
merged 3 commits into from
Jan 10, 2022
Merged

Conversation

liuzhe-lz
Copy link
Contributor

@liuzhe-lz liuzhe-lz commented Jan 10, 2022

Description

As of now the latest version of PyYAML (6.0) does not support YAML 1.2, which means it is not fully compatible with JSON.
This has caused multiple problems for NNI: #4450 #4452

The PR partially fixed this bug by trying YAML and JSON loaders consecutively. However if a user uses comment and scientific notation at same time, NNI will still fail.
Since PyYAML has already been working on YAML 1.2 support, I prefer to wait for its update for a perfect fix.

As a reminder, ruamel.yaml supports YAML 1.2, but according to our previous experience it's pip package and conda package have inconsistent APIs, which caused even more problems.

Checklist

  • test case
  • doc

How to test

Create tricky JSON search space file and make sure it's loaded correctly.

try:
return json.loads(content)
except Exception:
return yaml.safe_load(content)
Copy link
Contributor

Choose a reason for hiding this comment

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

so the yaml format search space still does not support scientific notation, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't.

@liuzhe-lz liuzhe-lz merged commit 31f11f5 into microsoft:master Jan 10, 2022
@liuzhe-lz liuzhe-lz deleted the json-ss branch January 10, 2022 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants