-
Notifications
You must be signed in to change notification settings - Fork 517
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
Bug/sg 000 merge failure for datasetparams #1140
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering into how many unexpected things we will run with this config merging, but..
LGTM )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ilure_for_datasetparams
…asetparams' into bug/SG-000_merge_failure_for_datasetparams
a451d97
…ilure_for_datasetparams # Conflicts: # src/super_gradients/training/utils/checkpoint_utils.py
Pls add test to tests/unit_tests/detection_dataset_test.py ^_^ |
Added a simple test that uses dataloaders.get with dataset_params that indclude an entry which is not present in the yaml file (this is what caused the crash). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Recent changes in #1092 caused pose estimation recipe tests to crash https://app.circleci.com/pipelines/github/Deci-AI/super-gradients/7748/workflows/5d210255-4bed-4736-b4d4-b693d6bc91e1/jobs/23382
This is becaue there are overrides for dataset params from the command line, causing the merge to fail.
Before calling merge we neeed to set the two configs's struct to false, other wise if one includes an attribute which the other one does not have - the merge will fail.