Skip to content

[Priority] [Bug] removing hardcoded local file paths and standardizing them#97

Open
Varun-sai-500 wants to merge 24 commits intoyoxu515:mainfrom
Varun-sai-500:small
Open

[Priority] [Bug] removing hardcoded local file paths and standardizing them#97
Varun-sai-500 wants to merge 24 commits intoyoxu515:mainfrom
Varun-sai-500:small

Conversation

@Varun-sai-500
Copy link
Copy Markdown
Contributor

@Varun-sai-500 Varun-sai-500 commented Apr 15, 2026

This PR modernizes configuration handling and data transformation utilities to improve compatibility, maintainability, and robustness across training and deployment pipelines.

Key Changes:

  1. Path Handling & Configuration Refactor
    Introduced BASE_DIR and standardized absolute path construction across configs
    Replaced hardcoded relative paths with os.path.join + os.path.abspath
    Refactored dynamic imports using f-strings
    Consolidated directory structure initialization using a common ROOT
    Updated logging paths:
    DIR_TB_LOG now derives from DIR_LOG
    Simplified directory creation with exist_ok=True

  2. Model Configuration Improvements
    Updated all MODEL_ENCODER_PRETRAIN paths to use absolute resolution
    Ensured consistent path handling across all model configs
    Reordered initialization where necessary to avoid dependency issues

  3. Semantic Cleanup
    Replaced sentinel values:
    TRAIN_LONG_TERM_MEM_GAP = 9999 → None
    TEST_LONG_TERM_MEM_GAP = 9999 → None
    Improves clarity and avoids implicit magic-number behavior

Important:
This refactor actually solves a problem because normal running the file (without flags) and it will just take it from here, and these configs have those vos02, it's incredibly harder to debug, so standardized to dataset paths
One noticeable change is I have removed data_wd and VOS2 from path because it really doesn't belong the project repository, it's local, slipped into production.

@Varun-sai-500
Copy link
Copy Markdown
Contributor Author

@z-x-yang Can you please review?

@z-x-yang
Copy link
Copy Markdown
Collaborator

Thanks — this PR is much smaller than #96 and the direction is reasonable, but I don’t think it is merge-ready yet because the new path resolution is currently incorrect.

Main blockers:

  1. In configs/default.py, BASE_DIR is already <repo>/configs, but ROOT = os.path.abspath(os.path.join(BASE_DIR, '..', '..')) resolves to the parent directory of the repo, not the repo root. When I instantiate the config, DIR_ROOT / DIR_DATA point outside the checkout, so datasets, results, logs, and checkpoints would be written/read from the wrong place.

  2. In the model configs, BASE_DIR is <repo>/configs/models, but paths like:
    os.path.join(BASE_DIR, '..', 'pretrain_models', ...)
    resolve to <repo>/configs/pretrain_models/..., not <repo>/pretrain_models/....

So the current refactor would break config-driven training/eval by looking for datasets and pretrained encoder weights in the wrong locations.

The None change for *_LONG_TERM_MEM_GAP looks directionally fine with the engine guard, but it should be tested after the path issues are fixed.

Suggested fix: define a single repo root helper once, e.g. from configs/default.py, and use the correct number of .. levels consistently. Then smoke-test one training config and one eval/demo config to print the resolved paths before merging.

@Varun-sai-500
Copy link
Copy Markdown
Contributor Author

Varun-sai-500 commented Apr 26, 2026

fixed those issues, must be good by now @z-x-yang

@Varun-sai-500 Varun-sai-500 changed the title Smaller Refactors Config related refactors Apr 26, 2026
@Varun-sai-500 Varun-sai-500 changed the title Config related refactors Bug: removing local file paths and standardizing them Apr 26, 2026
@Varun-sai-500 Varun-sai-500 changed the title Bug: removing local file paths and standardizing them Bug: removing hardcoded local file paths and standardizing them Apr 27, 2026
@Varun-sai-500 Varun-sai-500 changed the title Bug: removing hardcoded local file paths and standardizing them [Priority] [Bug] removing hardcoded local file paths and standardizing them Apr 30, 2026
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.

2 participants