Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sweep: Parameter value written w/ scientific notation gets converted to str? #1129

Open
EricCousineau-TRI opened this issue Jul 1, 2020 · 9 comments
Labels
c:sweeps Component: Sweeps ty:bug type of the issue is a bug

Comments

@EricCousineau-TRI
Copy link
Contributor

EricCousineau-TRI commented Jul 1, 2020

wandb --version && python --version && uname

  • Weights and Biases version: wandb, version 0.9.1
  • Python version: Python 3.6.9
  • Operating System: Linux (Ubuntu 18.04)

Description

I get the following error when attempting to launch a local sweep:

$ wandb sweep --controller ./sweep.yaml
...
  File ".../lib/python3.6/site-packages/wandb/sweeps/params.py", line
 118, in __init__
    if self.min >= self.max:
TypeError: '>=' not supported between instances of 'str' and 'float'

What I Did

I wrote a sweep.yaml file, following the documentation:
https://docs.wandb.com/sweeps/configuration#parameters

I launched as described above, but got the above error. When editing the code, I found that one of the parameters was getting converted to a string.

Reproduction:

( set -eux;
cd $(mktemp -d)

python3 -m venv .
source ./bin/activate

cat > requirements.txt <<'EOF'
wandb==0.10.18
numpy==1.19.0
scipy==1.5.0
EOF

pip install -r ./requirements.txt

cat > ./script.py <<'EOF'
import os
import wandb

def main():
    os.environ["WANDB_MODE"] = "dryrun"
    config = dict(
        lr=1e-4,
    )
    wandb.init(
        config=config,
    )
    print(wandb.config)

if __name__ == "__main__":
    main()
EOF

cat > ./sweep.yaml <<'EOF'
program: ./script.py

controller:
  type: local

method: bayes
metric:
  name: loss/train
  goal: minimize
parameters:
  lr:
    distribution: uniform
    min: 1.e-5  # This is the problem.
    max: 0.001

command:
  - ${env}
  - ${interpreter}
  - ${program}
  - ${args}
EOF

wandb sweep --controller ./sweep.yaml
)

Guesses

Something is randomly trying to stringify things?

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.85. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@issue-label-bot issue-label-bot bot added the ty:bug type of the issue is a bug label Jul 1, 2020
@EricCousineau-TRI
Copy link
Contributor Author

EricCousineau-TRI commented Jul 1, 2020

FTR, in the same virtualenv, PyYAML==5.3.1 parses this correctly:

$ python3 -c 'import yaml; print(yaml.safe_load("value: 1.0e-5"))'
{'value': 1e-05}

@vanpelt
Copy link
Contributor

vanpelt commented Jul 2, 2020

Thanks for reporting @EricCousineau-TRI we're digging into the root cause.

@ariG23498
Copy link
Contributor

Hey @EricCousineau-TRI
In the past few months we've majorly reworked the CLI and UI for Weights & Biases. We're closing stale issues. Please comment to reopen. 😄

@timxzz
Copy link

timxzz commented Feb 10, 2021

This problem still exists with version: 0.10.18

@EricCousineau-TRI
Copy link
Contributor Author

Aye, confirmed. Updated min-repro above

@ariG23498 Can we re-open this?

@ariG23498 ariG23498 reopened this Feb 11, 2021
@timxzz
Copy link

timxzz commented Feb 16, 2021

The root cause is indeed as @EricCousineau-TRI said can be PyYAML. Here is a link from stackoverflow addressing an workaround: https://stackoverflow.com/questions/30458977/yaml-loads-5e-6-as-string-and-not-a-number

@EricCousineau-TRI
Copy link
Contributor Author

EricCousineau-TRI commented Apr 2, 2021

From above, it seems that there is still a PR lingering for this builtin json <-> PyYAML yaml incompatibility.
Tracing through @timxzz's SO post, it seems there is an upstream issue and PR to fix this for PyYAML:

FWIW In my code, I simply just hack around it. I'm using dataclasses for schema-like stuff (similar to Hydra, I think?), and when I come across float, I have to do stuff like this:

def _maybe_convert(value, T):
    try:
        return T(value)
    except ValueError:
        return value

def _reconcile(value, T):
    ...
    assert isinstance(T, type), (T, value)
    if T == float:
        if isinstance(value, str):
            # Workaround:
            # https://github.com/wandb/client/issues/1129
            # https://github.com/yaml/pyyaml/issues/173
            value = _maybe_convert(value, float)
        assert isinstance(value, (float, int)), (T, value)
    ...

@EricCousineau-TRI
Copy link
Contributor Author

EricCousineau-TRI commented Apr 2, 2021

My suggested workaround for wandb is to convince the JSON emitter to generate YAML 1.1-friendly floats, e.g. instead of 5e-05, generate 5.0e-5.

Rationale: See this comment: yaml/pyyaml#173 (comment)

Since pyyaml implements YAML 1.1, this behaviour is actually correct.
It would be better to implement resolvers and representers for YAML 1.2 and let users choose the version.

@sydholl sydholl added the c:sweeps Component: Sweeps label Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:sweeps Component: Sweeps ty:bug type of the issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants