search

kraina-ai / srai

Spatial Representations for Artificial Intelligence - a Python library toolkit for geospatial machine learning focused on creating embeddings for downstream tasks
https://kraina-ai.github.io/srai/
Apache License 2.0
220 stars 17 forks source link

Add option to pass number of workers to torch dataloaders #456

Open RaczeQ opened 5 months ago

RaczeQ commented 5 months ago

Currently dataloaders don't use any kwargs and work in single-threaded mode. Add option to use not only **trainer_kwargs but also **dataloader_kwargs.

sabman commented 3 months ago

Quick question about this. Do we want to maintain backwards compatibility with a deprecation warning or shall we change the API to change the constructor signature by e.g. removing batch_size as an argument to dataloader_kwargs. (See some initial work here https://github.com/kraina-ai/srai/pull/464)

My personal take on it is to maintain backwards compatibility and have deprication warnings now with a breaking release later using semantic versioning.

Happy to take your lead on this.

simonusher commented 2 months ago

Hi @sabman! First of all, thank you for working on this.

As for maintaining backwards compatibility, it hasn't been our top priority, but we haven't fully ignored it either. I think we might have had at least a couple of breaking changes on minor versions as "this is new, we're moving fast and things may break". Mostly because we didn't know how people would use the library (and they'd want to use it), and if the API we had designed made sense. But if you're willing to add a deprecation warning then we'd appreciate this and we can maintain it up to the next major. Imo it's not a huge change, so we can do without it.

So in short: if you feel it's worth to add a deprecation warning then go for it, we'll appreciate it. If not then we can release the next minor with it, not fully following semantic versioning yet.