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

DRAFT: split dlc schema into train, model, pose #3

Merged
merged 10 commits into from
Apr 15, 2022
Merged

Conversation

CBroz1
Copy link

@CBroz1 CBroz1 commented Mar 11, 2022

WIP. Does this split of schema roles work for you, @ttngu207? If so, I'll update documentation to reflect the split.

See corresponding workflow PR, specifically the pipeline which now includes VideoRecording.

forthcoming updates:

  • Attempt full workflow usage, resolve issues with pieces that connect schemas.
  • documentation for usage, including upstream dependencies

Copy link
Owner

@ttngu207 ttngu207 left a comment

Choose a reason for hiding this comment

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

I think the split should be into two modules

  • model and pose-estimation
  • model training (which must import the other module)

I think model and pose-estimation should really go together, it wouldn't be much of a deeplabcut element with just model

@CBroz1
Copy link
Author

CBroz1 commented Mar 14, 2022

My motivation for splitting model and pose was the imagined use-case where there is only one model. A user could supply their own model table with only the project_path field and pose estimation would still function the same.

I expect...

  1. Every user will want to do pose estimation and therefore use the pose schema
  2. A subset will want to also do model evaluation and therefore need the full model schema
  3. A further subset will also want to launch training and therefore use all 3

In the ultimate version of this schema, will we still support the current 'run from on-disk files' process? Or will we want to push users away from that standard? If the former, it seems to me like 'only use the schema you need' is within the a-la-carte design philosophy of the elements.

@ttngu207
Copy link
Owner

The way the pose and model is implemented here is quite interconnected. I.e. to use pose, users must use our model, I'm not sure if they can just import and use pose by supplying their own model table and the pose estimation step can work.

@CBroz1
Copy link
Author

CBroz1 commented Mar 18, 2022

oh - you're right. I previously though that we only relied on project_path, but I took another look at do_pose_estimation where we require the model table to have shuffle, trainingsetindex, and model_prefix - I'll revert to 2 schemas and request your review when complete

CBroz1 and others added 8 commits March 18, 2022 15:04
helper funcs: fixed bug where root_dir[0] would get appended to root_dirs whenever
              get_processed_dir was called.
extract_new_bodyparts: added verbose arg to prevent duplicate print when calling
                       model.Model.insert_new_model
insert_new_model: because `BodyPart.extract_new_body_parts` returns a list, checking the
                  truth value is ambiguous. Changed to 'is not None'
infer_ouput_dir: remove _linking_module for VideoRecording table
insert_new_params: Enforce int type on paramset_idx input. Previously failed to return
                   when attempting to insert a duplicate bc str(x)!=int(x)
Co-authored-by: Thinh Nguyen <thinh@vathes.com>
@ttngu207 ttngu207 merged commit b067e9f into ttngu207:main Apr 15, 2022
ttngu207 pushed a commit that referenced this pull request Nov 15, 2023
ttngu207 added a commit that referenced this pull request Sep 20, 2024
Use `output_dir` as Path object in `save_yaml`
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