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.

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

Fixed model loading. #314

Merged
merged 2 commits into from
May 17, 2022
Merged

Fixed model loading. #314

merged 2 commits into from
May 17, 2022

Conversation

Spiess
Copy link
Member

@Spiess Spiess commented May 17, 2022 •
edited

  • Ensuring models are only loaded once through synchronized static methods.
  • Ensuring the SkeletonPose model is only loaded when needed for extraction and not for retrieval.

Closes #303.
Closes #311.

@Spiess
Fixed model loading.
5c07ee2 
- Ensuring models are only loaded once through synchronized static methods.
- Ensuring the SkeletonPose model is only loaded when needed for extraction and not for retrieval.

Addresses issues #303 and #311.
@Spiess
Bump version.
4389bdb
@Spiess Spiess requested a review from silvanheller May 17, 2022 15:18
lucaro
lucaro reviewed May 17, 2022
View reviewed changes
Copy link
Member

@lucaro lucaro left a comment

Choose a reason for hiding this comment

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

While model loading is synchronized, model access is not. Doesn't it cause problems if multiple threads are trying to evaluate the model simultaneously?

@Spiess
Copy link
Member Author

Spiess commented May 17, 2022

Good question, I just tried to cause a simultaneous execution and while I can't guarantee that the model access happened at the same time, results were consistent and no errors were thrown.
I was not able to find documentation whether or not it is possible to execute a TensorFlow model in parallel.

@lucaro
Copy link
Member

lucaro commented May 17, 2022

Ok, then let's assume it works like that until demonstrated otherwise.

silvanheller
silvanheller reviewed May 17, 2022
View reviewed changes
Copy link
Member

@silvanheller silvanheller left a comment

Choose a reason for hiding this comment

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

I'll test parallelism in the next days and merge this PR once the tests look good

silvanheller
silvanheller approved these changes May 17, 2022
View reviewed changes
Copy link
Member

@silvanheller silvanheller left a comment

Choose a reason for hiding this comment

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

My Evaluation code runs through with no issues, so this looks good to me. Thanks!

@silvanheller silvanheller merged commit 130a9f4 into master May 17, 2022
@silvanheller silvanheller deleted the fix/model-loading branch May 17, 2022 15:51
silvanheller pushed a commit that referenced this pull request Jun 10, 2022
@Spiess
Fixed model loading (#314)
47cc78a 
Former-commit-id: 130a9f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucaro lucaro lucaro left review comments

@silvanheller silvanheller silvanheller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Model-Loading in VisualTextCoEmbedding is not Thread-Safe Inefficient SkeletonPose TensorFlow model loading
3 participants
@Spiess @lucaro @silvanheller