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

[MNT] manage core dependencies prior to 1.1.0 release #1616

Closed
fkiraly opened this issue Aug 25, 2024 · 5 comments
Closed

[MNT] manage core dependencies prior to 1.1.0 release #1616

fkiraly opened this issue Aug 25, 2024 · 5 comments
Labels
maintenance Continuous integration, unit testing & package distribution

Comments

@fkiraly
Copy link
Collaborator

fkiraly commented Aug 25, 2024

Before we release 1.1.0, we should make sure we have managed the core dependencies carefully.

For instance, we do not want to add core dependencies that can be avoided, and potentially remove core dependencies that can be isolated.

Things to ensure we watch out for:

  • current fixes to CI added core dependencies - optuna-integration, pyarrow, tensorflow, cpflows.
    • optuna-integrations is required only if optuna>=3.3.0
  • downwards compatibility of depedency sets, we should not remove mqf2
  • bounds compatibility
@fkiraly fkiraly added the maintenance Continuous integration, unit testing & package distribution label Aug 25, 2024
@fkiraly
Copy link
Collaborator Author

fkiraly commented Aug 31, 2024

@benHeid, @yarnabrina, @fnhirwa, @XinyuWuu, question: what should be the end state of core/soft dependencies?

We should discuss soft dependencies and potential soft dependencies:

  • optuna, statsmodels - used only in the tuner, very localized scope of coupling; the dependency locus requires optuna-integration from optuna 3.3.0 onwards
  • matplotlib - heavy dependency, used in some loggers. Can be turned off
  • pytorch_optimizer - single-locus, used only for string aliasing of class-compatible optimizer. Could be easily removed.
  • pre-existing softdep sets graph, mqf2, github-actions
  • scikit-learn, pandas - less localized, more central
  • scipy - imported only twice for special scipy.stats, but an indirect dep of scikit-learn

For completeness, I consider the following scope-central and thus not advisable to move to soft dependencies, but we could also discuss - e.g., if we want to go with strict containerization:

  • torch, lightning
  • numpy

@fkiraly
Copy link
Collaborator Author

fkiraly commented Aug 31, 2024

My suggested end state for the 1.1.0 release:

core = torch, lightning, scikit-learn, pandas, scipy

all_extras = optuna, optuna-integration, statsmodels, matplotlib, pytorch_optimizer

existing dep sets graph, mqf2, github-actions remain unchanged

@benHeid
Copy link
Collaborator

benHeid commented Sep 3, 2024

I agree to your proposed end state.

@fnhirwa
Copy link
Member

fnhirwa commented Sep 4, 2024

My suggested end state for the 1.1.0 release:

core = torch, lightning, scikit-learn, pandas, scipy

all_extras = optuna, optuna-integration, statsmodels, matplotlib, pytorch_optimizer

existing dep sets graph, mqf2, github-actions remain unchanged

This makes sense to me also

fkiraly added a commit that referenced this issue Sep 4, 2024
Isolates `optuna`, `optuna-integration`, and `statsmodels` as soft dependencies in a new soft dep set `all_extras` to collect all soft dependencies, and in another soft dep set `tuning`. Towards #1616

This was quite simple, since the imports happen in only one very narrow location, the `optimize_hyperparameters` utility which is also not core architecturally.
fkiraly added a commit that referenced this issue Sep 4, 2024
Isolates `matplotlib` as soft dependency in a new soft dep set `all_extras`. #1616

The imports happen in `plot_sth` methods throughout the code base, some attached to classes, some not.
This allows to use `pytorch-forecasting` without plotting or graphical logging, or use a different plotting backend manually.

Isolation strategy:

* where the purpose of the function is creating a plot or nothing else, absence of `matplotlib` raises an exception
* where `matplotlib` is additional or part of the logic, absence of `matplotlib` causes the specific parts to be skipped. Example: `log_gradient_flow` - this is crucial, as raising an exception would prevent the models from running.
fkiraly added a commit that referenced this issue Sep 4, 2024
Isolates `pytorch-optimizer` as soft dependency in a new soft dep set `all_extras`. #1616

The imports happen only in `BaseModel`, when resolving aliases for `optimizer`.

Isolation consists of two steps:
* replacing resolution of the alias with a request to install the package, and removing from resolution scope if not installed
* replacing the default optimizer `"ranger"` with `"adam"` if `pytorch-optimizer` is not installed (left at `"ranger"` if installed)

Deprecation messages and actions are added, to changne the default to `"adam"` from 1.2.0, in order to minimize the number of dependencies in default parameter settings
@fkiraly
Copy link
Collaborator Author

fkiraly commented Sep 8, 2024

done with 1.1.0 release

@fkiraly fkiraly closed this as completed Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution
Projects
None yet
Development

No branches or pull requests

3 participants