Move domain-specific functionality to subpackages

FluxML / FastAI.jl

Repository of best practices for deep learning in Julia, inspired by fastai

https://fluxml.ai/FastAI.jl

MIT License

585 stars 51 forks source link

Move domain-specific functionality to subpackages #240

Closed lorenzoh closed 2 years ago

lorenzoh commented 2 years ago

This turns the domain-specific modules into packages as discussed in #186. Specifically:

FastAI.Vision -> FastVision
FastAI.Tabular -> FastTabular
FastAI.Textual -> FastText
FastMakie for Makie backend

This is obviously a gigantic breaking change and I'm thinking to tag this together with #196 to avoid subsequent breaking changes. Thoughts @darsnack? This should also be the last of the larger (and breaking) changes I have been planning to make, so if there are any other breakages planned, we should tag them together.

Closes #224, closes #241

Status:

[x] create subpackages
[x] move and fix tests
[x] add test projects for subpackages
[x] create CI for subpackages
[x] apply JuliaFormatter (SciMLStyle)
[x] update documentation to work with subpackages
- [x] update markdown sources and docstrings
- [x] rerun notebooks

darsnack commented 2 years ago

I think based on our experience with NNlib, the subpackage approach seems to be the best option moving forward. The only other possible breaking change I can think off is the data blocks API for time series support.

darsnack commented 2 years ago

I tried doing a review, but I am confused about the final organization. Will a user load FastAI or FastVision? Right now, I don't see how FastAI requires FastVision, for example.

lorenzoh commented 2 years ago

FastAI is now a core package, with FastVision etc. providing the domain-specifc functionality. So to run computer vision tasks, one imports using FastAI, FastVision. Loading the domain packages also populates the feature registries (datasets, datarecipes and learningtasks). With more domains being added, reducing the number of total packages that have to be loaded helps reduce the import time.

I am afraid the magnitude of changes and the automatic formatting will make a proper review hard, so feel free to focus on the big-picture changes

lorenzoh commented 2 years ago

The only other possible breaking change I can think off is the data blocks API for time series support.

What would be breaking about that?

darsnack commented 2 years ago

Maybe this has been resolved in more recent discussions. I recall that we mentioned how for sequential data, each sample is a sequence of features. So the outermost list is over samples like any dataset. But for Flux models, the outermost list is over time steps, and each element is a batch of features at a given time step. This means that there is an "encoding" (i.e. MLUtils.batchseq) that needs to happen after batching. It's definitely a solvable issue, but I didn't know how you wanted to handle it. Potentially the solution would be breaking or non-breaking.