Skip to content

Fix bug dataset and metadataset fixes DeepLabCut/DeepLabCut#2390 #2405

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.

Sign up for GitHub

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

Jump to bottom
Merged
MMathisLab merged 14 commits into from
Dec 7, 2023
Merged

Fix bug dataset and metadataset fixes DeepLabCut/DeepLabCut#2390 #2405

MMathisLab merged 14 commits into from
Dec 7, 2023

Conversation

@Tetra-quark
Copy link
Contributor

@Tetra-quark Tetra-quark commented Oct 5, 2023 •
edited
Loading

Pull request to fix issue #2390

TODOs:

  • Fix getting pose_cfg paths in nested loop over shuffle and trainingfractions. Needs to be able to handle trainingsetindex='all'. Fix needs to be applied to evaluate.py and multianimal_evaluate.py.
  • Look for other usages of get_data_and_metadata_filenames in other areas of the project and evaluate whether to replace with these with simple dict access from pose_cfg. (For file creation the method should be used but I think afterwards ideally it should be accessed from pose_cfg).
  • Rename variable dlc_cfg to test_pose_cfg to be more specific and mirror the naming of train_pose_cfg
  • Handle exception if train/pose_cfg.yaml is not found similarly to how load test config is in a try/catch block.
  • Update visualizemaps.py

Tetra-quark and others added 4 commits September 22, 2023 11:07
@Tetra-quark
Refactor metadataset usage in calculatepafdistancebounds function
b7440e1 
- Remove unsued dataset path.
- Fix getting metadataset. Now obtained from config rather than auxiliary function.
- Suppress unused return values from load_metadataset.
- Move load_metadata to where it is explicitly needed.
@Tetra-quark
Fix metadataset file path in return_evaluate_network_data function
e1f1d17 
- Get metadataset file path from config rather than auxiliary function.
- Use return_train_network_path function to get train/test pose_cfg paths
- Move load_metadata to after try-catch block.
@Tetra-quark
Fix metadataset file path in return_evaluate_network_data function
d1d646c 
- Get metadataset file path from config rather than auxiliary function.
- Suppress redefined TrainFraction from load_metadata function.
- Use return_train_network_path function to get train/test pose_cfg paths
- Move load_metadata to after try-catch block.
- Fix minor naming/formatting issues
@Tetra-quark
Merge branch 'DeepLabCut:main' into fix_bug_dataset_and_metadataset
1b1fea6
@Tetra-quark Tetra-quark changed the title Fix bug dataset and metadataset fixes #2390 Fix bug dataset and metadataset fixes DeepLabCut/DeepLabCut#2390 Oct 5, 2023
@MMathisLab MMathisLab requested a review from jeylau October 6, 2023 13:28
@AlexEMG
Copy link
Member

AlexEMG commented Oct 9, 2023

Thanks!

@AlexEMG AlexEMG requested a review from n-poulsen October 9, 2023 20:49
@Tetra-quark
Update naming: dlc_cfg -> test_pose_cfg
2f586b4
@Tetra-quark
Update naming: dlc_cfg -> test_pose_cfg
086578a
@Tetra-quark
Update evaluate.py
ad919a1 
- Fix handling of trainingsetindex="all"
- Replace Try/Except on test/pose_cfg with check if modelfolder exists so it covers both train and test pose_cfgs.
- Add TODO related to FileNotFoundError
- Minor Refactoring
@Tetra-quark
Update evaluate_multianimal.py
ba521c2 
- Fix handling of trainingsetindex="all"
- Replace Try/Except on test/pose_cfg with check if modelfolder exists so it covers both train and test pose_cfgs.
- Add TODO related to FileNotFoundError
- Minor Refactoring
@Tetra-quark
Copy link
Contributor Author

Tetra-quark commented Oct 17, 2023 •
edited
Loading

I have looked for other areas of the code that use get_data_and_metadata_filenames they are either related to dataset/metadataset file creation (no need to change these) or are test scripts where it matters less how these filenames are accessed.

However, visualizemaps.py should be updated given it's structural similiarity to evaluate.py and evaluate_multianimal.py. But before doing that I was hoping to get some feedback on the code review I opened above as it is relevant to all three files.

I also wanted to note explicitly that my changes now mean that the metadataset parameter in the train/pose_cfg.yaml is being accessed by modules that previously only accessed the test/pose_cfg.yaml. This is because while there is a dataset parameter in the test/pose_cfg.yaml as far as I can see from the testing scripts there is no metadataset parameter.

@Tetra-quark Tetra-quark marked this pull request as ready for review October 17, 2023 13:07
@MMathisLab MMathisLab added enhancement New feature or request documentation documentation updates/comments bug fix! fix for a real buggy one... labels Oct 27, 2023
jeylau
jeylau suggested changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@jeylau jeylau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Tetra-quark! I'd only ask you to load the train image indices outside the loop, and that looks great!

@Tetra-quark
Update evaluate.py (requested change)
32af2b0 
- Load config and metadata outside nested loop.
@jeylau
Copy link
Contributor

jeylau commented Oct 30, 2023

Great, thanks! I think this is good to merge, and if some code refactoring is needed in evaluate/visualizemaps, that can be done in a separate PR @MMathisLab @Tetra-quark.

n-poulsen
n-poulsen reviewed Oct 30, 2023
View reviewed changes
Copy link
Contributor

@n-poulsen n-poulsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor stye comments, otherwise looks good to me

@Tetra-quark Tetra-quark requested a review from n-poulsen December 2, 2023 15:12
@MMathisLab MMathisLab merged commit 0f657cd into DeepLabCut:main Dec 7, 2023
@Tetra-quark Tetra-quark deleted the fix_bug_dataset_and_metadataset branch December 30, 2023 18:38
@MMathisLab MMathisLab mentioned this pull request Feb 8, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@n-poulsen n-poulsen Awaiting requested review from n-poulsen

1 more reviewer

@jeylau jeylau jeylau approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jeylau jeylau

Labels

bug fix! fix for a real buggy one... documentation documentation updates/comments enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Tetra-quark @AlexEMG @jeylau @n-poulsen @MMathisLab