TUM-DAML / seml

SEML: Slurm Experiment Management Library
Other
165 stars 29 forks source link

Boolean configs get converted to integers #126

Closed isefos closed 7 months ago

isefos commented 8 months ago

When I specify boolean config values in my yaml file as such:

# ...
fixed:
  example1: true
  example2: false
# ...

these get cast to integers (1 and 0) during the add command (are stored as integers in MongoDB).

When they get loaded for the start command they are of course still integers. But I need them to be boolean and I don't really have an easy way of checking at runtime if the 0/1 is a valid integer value or should actually be converted back into a boolean.

I traced the cause of this conversion to config.py line 242:

# Cast NumPy integers to normal integers since PyMongo doesn't like them
all_configs = [
    {
        k: int(v) if isinstance(v, numbers.Integral) else v
        for k, v in config.items()
    }
    for config in all_configs
]

This casts bool values to int because bool is a subclass of int and isinstance(True, numbers.Integral) == True.

The comment above only mentions NumPy integers so I'm guessing this additional cast of boolean values is not intentional...

Explicitly checking for bool here would already solve this issue for me:

# Cast NumPy integers to normal integers since PyMongo doesn't like them
all_configs = [
    {
        k: int(v) if (isinstance(v, numbers.Integral) and not isinstance(v, bool)) else v
        for k, v in config.items()
    }
    for config in all_configs
]

This way boolean values stay bool and get saved to MongoDB and loaded that way as well.

n-gao commented 8 months ago

Thanks for the bug report! This will be fixed in https://github.com/TUM-DAML/seml/pull/125. I haven't gotten around to reviewing it. For now, you can checkout the branch and work with that if you need a fix right away.

n-gao commented 7 months ago

@isefos should be fixed now. Please confirm.