Skip to content

ruler os.environ.get fix #2180

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

f14-bertolotti
Copy link
Contributor

Motivation

Running the ruler benchmark (pencompass/configs/datasets/ruler/ruler_16k_gen.py) raised a model engine error related to the usage of os.environ.get(...). The traceback is as follows:

Traceback (most recent call last):
  File "/leonardo_work/iGen_train/fbertolo/llm-research/eval/../third_party/opencompass/run.py", line 4, in <module>
    main()
  File "/leonardo_work/iGen_train/fbertolo/llm-research/third_party/opencompass/opencompass/cli/main.py", line 259, in main
    cfg = get_config_from_arg(args)
  File "/leonardo_work/iGen_train/fbertolo/llm-research/third_party/opencompass/opencompass/utils/run.py", line 97, in get_config_from_arg
    config = Config.fromfile(args.config, format_python_code=False)
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/config.py", line 494, in fromfile
    raise e
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/config.py", line 492, in fromfile
    cfg_dict, imported_names = Config._parse_lazy_import(filename)
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/config.py", line 1081, in _parse_lazy_import
    _base_cfg_dict, _base_imported_names = Config._parse_lazy_import(  # noqa: E501
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/config.py", line 1081, in _parse_lazy_import
    _base_cfg_dict, _base_imported_names = Config._parse_lazy_import(  # noqa: E501
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/config.py", line 1109, in _parse_lazy_import
    exec(
  File "/leonardo_work/iGen_train/fbertolo/llm-research/third_party/opencompass/opencompass/configs/datasets/ruler/ruler_16k_gen.py", line 17, in <module>
    tokenizer_model = os.environ.get('TOKENIZER_MODEL', 'gpt-4')
  File "/leonardo_scratch/fast/iGen_train/fbertolo/eval/venv/lib/python3.10/site-packages/mmengine/config/lazy.py", line 205, in __call__
    raise RuntimeError()
RuntimeError

This error affects all ruler_<number>k_gen.py.

Modification

To resolve this, I replaced os.environ.get(...) with os.environ.build().get(...), which I have used successfully in similar model engine contexts. An alternative is to use the {{$ENV_NAME:value}} syntax, but this tends to introduce additional complexity and potential errors.

Checklist

Before PR:

  • Pre-commit or other linting tools have been used to fix potential lint issues.
  • Bug fixes are covered by unit tests, including the case that causes the bug.
  • Modifications are covered by complete unit tests.
  • Documentation has been updated accordingly (e.g. docstrings, tutorials).

After PR:

  • If the modification potentially influences downstream or related projects, it has been tested with those projects.
  • CLA has been signed, and all committers have signed the CLA for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants