Skip to content
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

feat: reco training script with multi paths sets #377

Merged
merged 5 commits into from
Jul 13, 2021
Merged

Conversation

charlesmindee
Copy link
Collaborator

This PR adds the option to train recognition with multiple datasets.
the arg data_path in the script is replaced by a train_data_path, a list of 1 or more paths and val_data_path, a path.
The structure inside each path must be: path/images/ and path/labels.json.
Any feedback is welcome!

@charlesmindee charlesmindee added type: enhancement Improvement ext: references Related to references folder labels Jul 12, 2021
@charlesmindee charlesmindee added this to the 0.3.1 milestone Jul 12, 2021
@codecov
Copy link

codecov bot commented Jul 12, 2021

Codecov Report

Merging #377 (982a1e6) into main (8e14332) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #377   +/-   ##
=======================================
  Coverage   96.25%   96.25%           
=======================================
  Files          83       83           
  Lines        3554     3554           
=======================================
  Hits         3421     3421           
  Misses        133      133           
Flag Coverage Δ
unittests 96.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e14332...982a1e6. Read the comment docs.

Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'm not sure passing a list to a bash script is a good idea. Say we have dozens of parts, it would be better to assume that all parts are the subfolders of train_set for instance (train_set/part1, train_set/part2, etc.). If the json labels are directly in this folder, then there is only 1 part, if not, we instantiate the set with the first part, then merge the others. What do you think?

references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_tensorflow.py Outdated Show resolved Hide resolved
references/recognition/train_tensorflow.py Outdated Show resolved Hide resolved
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

One last improvement suggestion and we're good to merge!

references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_pytorch.py Outdated Show resolved Hide resolved
references/recognition/train_tensorflow.py Outdated Show resolved Hide resolved
references/recognition/train_tensorflow.py Outdated Show resolved Hide resolved
fg-mindee
fg-mindee previously approved these changes Jul 12, 2021
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks again!

Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks!

@charlesmindee charlesmindee merged commit b5a7d86 into main Jul 13, 2021
@charlesmindee charlesmindee deleted the multis branch July 13, 2021 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: references Related to references folder type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants