search

pikarpov-LANL / Sapsan

ML-based turbulence modeling for astrophysics
https://sapsan-wiki.github.io
Other
13 stars 4 forks source link

Suggest to loosen the dependency on catalyst #163

Closed Agnes-U closed 1 year ago

Agnes-U commented 1 year ago

Dear developers,

Your project Sapsan requires "catalyst>=21.5" in its dependency. After analyzing the source code, we found that the following versions of catalyst can also be suitable without affecting your project, i.e., catalyst 21.4, 21.04, 21.04.1, 21.4.2, 21.04.2. Therefore, we suggest to loosen the dependency on catalyst from "catalyst>=21.5" to "catalyst>=21.4" to avoid any possible conflict for importing more packages or for downstream projects that may use ddos_script.

May I pull a request to further loosen the dependency on catalyst?

By the way, could you please tell us whether such dependency analysis may be potentially helpful for maintaining dependencies easier during your development?


Details:

Your project (commit id: 3b3d63a3ad02428651d5721d42213c1278dc8e03) directly uses 5 APIs from package catalyst.

catalyst.runners.runner.SupervisedRunner.__init__, catalyst.callbacks.scheduler.SchedulerCallback.__init__, catalyst.engines.torch.DeviceEngine.__init__, catalyst.callbacks.checkpoint.CheckpointCallback.__init__, catalyst.callbacks.misc.EarlyStoppingCallback.__init__

Beginning fromwhich, 8 functions are then indirectly called, including 5 catalyst's internal APIs and 3 outsider APIs as follows:

[/pikarpov-LANL/Sapsan]
+--catalyst.runners.runner.SupervisedRunner.__init__
|      +--catalyst.runners.supervised.ISupervisedRunner.__init__
|      |      +--catalyst.core.runner.IRunner.__init__
|      |      |      +--collections.defaultdict
|      +--catalyst.runners.runner.Runner.__init__
|      |      +--catalyst.core.runner.IRunner.__init__
+--catalyst.callbacks.scheduler.SchedulerCallback.__init__
|      +--catalyst.core.callback.Callback.__init__
+--catalyst.engines.torch.DeviceEngine.__init__
+--catalyst.callbacks.checkpoint.CheckpointCallback.__init__
|      +--catalyst.core.callback.Callback.__init__
|      +--catalyst.tools.metric_handler.MetricHandler.__init__
|      |      +--functools.partial
+--catalyst.callbacks.misc.EarlyStoppingCallback.__init__
|      +--catalyst.callbacks.misc.IEpochMetricHandlerCallback.__init__
|      |      +--abc.ABC.__init__
|      |      +--catalyst.tools.metric_handler.MetricHandler.__init__

Since all these functions have not been changed between any version for package "catalyst" from [21.4, 21.04, 21.04.1, 21.4.2, 21.04.2] and 21.5. Therefore, we believe it is safe to loosen the corresponding dependency.

pikarpov-LANL commented 1 year ago

Hi, thanks for bringing it to my attention! Catalyst, indeed, has been giving me compatibility problems since their team decided to change internal functions not too long ago. Currently, catalyst>21.5 breaks logging in torch_backend.py, given some parameters are no longer attributes of the catalyst objects Sapsan relies on. I do plan to get Sapsan up-to-date with the latest Catalyst version in the next release (0.7.0).

Your analysis is certainly useful, but I don't think it would be meaningful to only slightly loosen the package requirement right now, given the upcoming change in compatibility with the latest Catalyst 22.04.

Since I am only focused on Catalyst at the moment, please feel free to let me know and submit a pull request to loosen any other package dependency!