load_pretrained_model dependency issues

[IFenton]

I've been working on trying to get the Scivision example gallery notebooks running (#83, and I've come across an issue with load_pretrained_model that is causing some problems.

Running load_pretrained_model(<url>, allow_install=True), if the model is not present, currently force installs both the model and its dependencies. As I understand it, this was intended as an intentional feature to make sure that the dependencies were updated to the required versions (#223). However the force reinstalling of dependencies is leading to issues with some of the scivision-examples-gallery notebooks (e.g. scivision-basic-usage-example #5, plant-phenotyping-classification #8). In both these cases, running load_pretrained_model causes the latest version of packages (tensorflow and timm respectively) to be installed. However, these versions are causing the errors in running the notebooks.

There seem to be a couple of possible options to fix this:

• Install the model separately, whether in the environment.yml or in the terminal before running the notebook
• Update the model requirements.txt to a pinned version of the package
• Change load_pretrained_model so it doesn't force reinstall dependencies by default

Does anyone have any preferences on which is the best option?

[edwardchalstrey1]

This is a good question, and a bit of a fundamental one for scivision! My opinion has been that installing models via the scivision package is too inherently problematic for reasons like this and we should no longer support this - see #346 (which proved controversial).

I think the tension here is we could either:

1. Make the postdocs notebooks just work somehow, e.g. one of @IFenton first two bullet points
2. Take some time to consider what scivision "best practice" should be for installing models in a Python environment, then conform all the project notebooks to this way of working (which could still be the first option, via enironment.yml)

[edwardchalstrey1]

There could even be a rule like "We only accept models for the scivision catalog that have pinned package versions" or as @ots22 has discussed previously, some kind of installation checks that run in GitHub actions

[ots22]

There are a few general issues with model-loading-as-package-installation as noted by Ed. Fixing this bug should at least let us maintain a reproducible environment without load_pretrained_model clobbering it, even without addressing these wider issues, so some comments in that spirit.

It might not be appropriate to completely pin dependencies in a model repository (or use a lockfile from the model if one exists) for the usual reasons - the right place to pin things would seem to be in the environment.yml in the gallery example. A model should ensure that its dependencies are specified correctly.

We should pin the version of the model itself in the gallery example. The options for doing that are:

1. as part of the load_pretrained_model command (e.g. include the commit hash when loading it)
2. in environment.yml (in this case, allow_install would not need to be set when loading the model)

We should allow for either possibility if we can: option 2 means someone running the model doesn't have the installation appearing inline, and option 1 if we want to explicitly show off this behaviour. (Some guidance on this could be useful to capture perhaps something like - when working interactively you might want to use allow_install=True but as soon as you start specifying dependencies in a virtual environment of some kind you should include your models there too - I'm not sure if that's right, but probably for a separate discussion.)

It seemed like (from our discussion the other day) the 'force' flag of pip is inherited when installing dependencies and this is the real culprit, since it is then no longer possible to use option 1 above (it might result in an 'upgraded' version of the package for instance, different from the version in environment.yml). Have you tried passing the 'force' flag to pip only when allow_install='force'? I think we had assumed that there was no downside to this flag if the package wasn't installed.

So I'd say it would be nice to end up with:

• load_pretrained_model(..., allow_install=True) not clobbering versions of other packages in the environment if the environment is consistent with the model dependencies
• having pinned versions of everything in the gallery examples, including the model - both options above should work - making a decision separately about which is preferable

[ots22]

Did #533 fix this?

(@IFenton)

[IFenton]

It fixed the specific problem of force updating to the latest versions, although I think the broader discussion of how to handle the dependencies are still unresolved

[ots22]

Okay, will close based on having fixed the force install problem.

Is there a summary of the broader problem somewhere?