TUM-DAML / seml

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

Use `repr` instead of `json.dumps` #53

Closed n-gao closed 2 years ago

n-gao commented 3 years ago

Reference issue

This is a follow-up PR to #51.

51 uses json.dumps which works reasonably fine in most scenarios and avoids a long range of single quotes in the outputted commands. However, JSON and Python code is not a 1:1 matching, e.g., boolean literals start with lower case in JSON (true/false) but with upper case in Python (True/False). This causes any parameter containing a boolean value to be returned as string by eval.

What does this implement/fix?

This PR contains two fixes:

Additional information

seml:
  executable: script.py
  name: test
  output_dir: ~/slurm-output
  conda_environment: seml_test
  project_root_dir: .

slurm:
  experiments_per_job: 1
  sbatch_options:
    gres: gpu:0
    mem: 1G
    cpus-per-task: 1
    time: 00-00:01
    partition: cpu
    qos: cpu

fixed:
  wfnet.test: True
import seml
from sacred import Experiment

ex = Experiment()
seml.setup_logger(ex)

@ex.config
def config():
    overwrite = None
    db_collection = None

    if db_collection is not None:
        ex.observers.append(seml.create_mongodb_observer(
            db_collection, overwrite=overwrite))

@ex.automain
def run(
    wfnet: dict,
    seed=None,
):
    return locals()

seml print-command

********** First experiment **********
Executable: script.py
Anaconda environment: seml_test

Arguments for VS Code debugger:
["with", "--debug", "wfnet={'test': True}", "config_hash='69ca8ced44256974e296284455dbabc6'", "db_collection='seml_test'", "--unobserved"]
Arguments for PyCharm debugger:
with --debug 'wfnet={'"'"'test'"'"': True}' 'config_hash='"'"'69ca8ced44256974e296284455dbabc6'"'"'' 'db_collection='"'"'seml_test'"'"'' --unobserved

Command for post-mortem debugging:
python script.py with 'wfnet={'"'"'test'"'"': True}' 'config_hash='"'"'69ca8ced44256974e296284455dbabc6'"'"'' 'db_collection='"'"'seml_test'"'"'' --unobserved --pdb

Command for remote debugging:
python -m debugpy --listen 172.24.64.17:35557 --wait-for-client script.py with 'wfnet={'"'"'test'"'"': True}' 'config_hash='"'"'69ca8ced44256974e296284455dbabc6'"'"'' 'db_collection='"'"'seml_test'"'"'' --unobserved

********** All raw commands **********
python script.py with 'wfnet={'"'"'test'"'"': True}' 'config_hash='"'"'69ca8ced44256974e296284455dbabc6'"'"'' 'db_collection='"'"'seml_test'"'"'' overwrite=19 --force

While the commands are certainly not beautiful they are at least correct.

n-gao commented 2 years ago

Is there any plan on merging this? :)

gasteigerjo commented 2 years ago

Absolutely! Once I find the time to test it... 😅

n-gao commented 2 years ago

This issue is quite severe. It essentially blocks you from using boolean variables or 2d arrays in your yaml files. It would be great if this could be merged or #51 be reversed.

danielzuegner commented 2 years ago

@n-gao does this fix #60? If yes you can close that issue and refer to this.