-
Notifications
You must be signed in to change notification settings - Fork 3k
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
PyTorch adapter: add a way to disable cache updates #5549
Conversation
df3c80b
to
03c767e
Compare
170fad2
to
5f507d3
Compare
@@ -42,6 +42,7 @@ def __init__( | |||
label_name_to_index: Mapping[str, int] = None, | |||
task_filter: Optional[Callable[[models.ITaskRead], bool]] = None, | |||
include_subsets: Optional[Container[str]] = None, | |||
update_policy: UpdatePolicy = UpdatePolicy.IF_MISSING_OR_STALE, |
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 suggest to allow CacheManager
instances too, especially in the TaskVisionDataset
class. This should simplify testing, using custom cache controllers, and, the most important, will allow to use a single cache controller in the multi-threaded scenario.
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 don't want to expose the CacheManager
interface to the public; it's pretty ad-hoc, and I didn't design it to be user-extensible.
and, the most important, will allow to use a single cache controller in the multi-threaded scenario.
Can you elaborate on this?
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.
Ok, I understand this. I've also double-checked the cache object calls, they seem to be safe. It's just that the users need to think if these operations are thread-safe or not.
5f507d3
to
a66a229
Compare
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.
👍
This will let users to run their PyTorch code without network access, provided that they have already cached the data. ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> Unit tests.
Motivation and context
This will let users to run their PyTorch code without network access, provided that they have already cached the data.
How has this been tested?
Unit tests.
Checklist
develop
branch[ ] I have updated the documentation accordingly[ ] I have linked related issues (read github docs)[ ] I have increased versions of npm packages if it is necessary (cvat-canvas,cvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.