facebookresearch / facebookresearch/sam2
Clarify build_sam2 config_file contract and support external config dirs
- Dominant language
- Jupyter Notebook
- Stars
- 19.9k
- Forks
- 2.5k
- PR merge metrics
- No merged PRs in 30d
Description
`build_sam2()` currently accepts an argument named `config_file` and passes it directly to Hydra as `config_name`:
https://github.com/facebookresearch/sam2/blob/2b90b9f5ceec907a1c18123530e92e794ad901a4/sam2/build_sam.py#L69-L92
```python
cfg = compose(config_name=config_file, overrides=hydra_overrides_extra)
```
SAM2 initializes Hydra with the `sam2` config module:
https://github.com/facebookresearch/sam2/blob/2b90b9f5ceec907a1c18123530e92e794ad901a4/sam2/__init__.py
```python
initialize_config_module("sam2", version_base="1.2")
```
That means `config_file` is not a generic filesystem path. It is a Hydra config name/path resolved inside Hydra's initialized config search path, for example:
```python
"configs/sam2.1/sam2.1_hiera_l.yaml"
```
This has confused users who pass absolute filesystem paths like:
```python
"/kaggle/working/models/sam2.1_hiera_l.yaml"
```
Related reports:
- facebookresearch/sam2#544
- facebookresearch/hydra#3033
- facebookresearch/hydra#3045
From Hydra's perspective, this is expected: `compose(config_name=...)` resolves a config name from the configured search path. It does not mean "load this arbitrary file path."
I think SAM2 should clarify this contract and, if external config files are supported, expose that explicitly. A clean API could accept an additional optional config directory/search path argument, then initialize or extend Hydra's config search path appropriately. Users would pass something like:
```python
build_sam2(
config_file="sam2.1_hiera_l.yaml",
config_dir="/kaggle/working/models",
ckpt_path=...,
)
```
or similar.
The important point is that `config_file` should remain a config name resolved inside the search path, while `config_dir` or an equivalent argument tells SAM2 where external configs live.
This avoids relying on the current double-slash workaround and gives users a clearer mental model:
- `config_dir`: where to search
- `config_file` / `config_name`: what config to compose from that search path
Can you clarify the intended contract for `build_sam2(config_file=...)` and consider adding an explicit external config directory option if loading user-provided config files is supported?
Contributor guide
Research direction
Start with sam2/build_sam.py, especially build_sam2() and its compose(config_name=...) call, then read sam2/__init__.py to understand Hydra initialization. Check the linked Hydra reports and existing configuration layout before choosing how an external search path should be exposed. Done means the config_file contract is documented and, if supported, an explicit external config directory works without the double-slash workaround.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100