feat: extend gym list and gym search commands - #2062
Conversation
gym list and gym search commands
330b60e to
a52eca1
Compare
|
/ok to test a52eca1 |
| from nemo_gym.model_registry import discover_models | ||
|
|
||
|
|
||
| def list_models() -> None: |
There was a problem hiding this comment.
Could we make sure gym list models and gym eval run use the same collision rule? list_models()/discover_models() let an earlier root replace a whole model group, while --model-type searches for an exact config in each root. This means a model type can disappear from gym list models even though it is still selectable.
E.g., if we have:
/tmp/my-plugin/
└── responses_api_models/
└── vllm_model/
└── configs/
└── vllm_model.yaml
And Gym includes:
responses_api_models/
└── vllm_model/
└── configs/
├── vllm_model.yaml
└── vllm_model_for_training.yaml
When running with:
gym list models --search-dir /tmp/my-plugin
vllm_model/vllm_model_for_training is not listed because the plugin's vllm_model entry replaces the whole built-in entry. But this still resolves the built-in config:
gym eval run \
--search-dir /tmp/my-plugin \
--model-type vllm_model/vllm_model_for_training \
...
Could discovery and selection either both apply precedence to the whole model group, or both apply it to each model type separately?
There was a problem hiding this comment.
fixed in e531be2 I also added test for the example you provided
a52eca1 to
3f48c4f
Compare
|
🌿 Preview your docs: https://nvidia-preview-martas-cli-1582-2-list-search.docs.buildwithfern.com/nemo/gym |
3f48c4f to
e531be2
Compare
|
It is also unclear what QUERY is really matched against. From the But matching is done wider. For example: Or But it is not matched against description |
|
Side note: we load a lot of modules to fire up a query. I takes about 10 seconds on a cold-start for me to get results. |
@prokotg this is expected behavior. module is optional and defaults to benchmarks, so your command is not interpreted as one with empty query, but as one with skipped module. |
|
Ticket for addressing the slowness: #2100 Includes likely cause and proposed solution |
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
|
@marta-sd I understand the mechanism but I think it's inconsistent. There is no default for |
Agreed this is inconsistent with
Given these two, I think it's better to stick to module being optional |
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Originally we've used the dataset name. These two are usually identical, but for benchmarks with non-standard config location the output from `gym list command` was not usable as value for `--benchmark` flag in other commands. This commit fixes it. Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
…er components and --model-type flag) Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
Signed-off-by: Marta Stepniewska-Dziubinska <martas@nvidia.com>
3aa0f84 to
ee0af30
Compare
|
/ok to test ee0af30 |
Extends CLI commands for discovery:
Part of #1582