search

icatcherplus / icatcher_plus

iCatcher+: Robust and automated annotation of infant gaze from videos collected in laboratory, field, and online studies
GNU General Public License v3.0
17 stars 17 forks source link

Sw/model cache #44

Closed samwinebrake closed 1 year ago

samwinebrake commented 1 year ago

Creates a cache in github actions for model directory

samwinebrake commented 1 year ago

Right now, version is hardcoded into workflow because pooch caches models in a directory w/ version number. Should update this in future.

yoterel commented 1 year ago

1) if needed the version can be automatically extracted by calling icatcher --version, so there should be no hard coded version anywhere in the workflow. 2) why is this version dependent ? you can set pooch to download the files to anywhere by setting an environment variable ICATCHER_DATA_DIR. 3) More importantly than the above, since the data files can and will be changed in the future, and this operation will be tied to a version release always, the cache should be updated (replaced!) in publish.yml workflow every release automatically. If this can't happen, we should let go of this idea completely.

samwinebrake commented 1 year ago
  1. True I can change this if we decide to keep this feature.
  2. Was just using the default save path since I liked the idea of having the model cache updated if the version changed, but could easily change to anywhere by setting the environment variable.
  3. I currently only have this in the test.yml workflow, just to make sure tests still run properly when changes are made to the codebase. Is this something that should be in the publish.yml workflow as well? I feel like this would be redundant but maybe I am thinking about the process wrong. Also, I don't think it would be a big deal to let go of the idea completely, it doesn't add much time to just download the models every run.

On another note, it might be a good idea to bring back hashes, but bring in models separately, not in a zip (so each model has its own hash). That way we only have to bring in whatever model is being tested, and can monitor when certain models are updated without having to change all model hashes. Let me know your thoughts on this.

yoterel commented 1 year ago

Let's give up on the idea for the time being as it just accelerates the workflow and doesn't actually affect it.

Regarding your last point, I think it is best not to bring back hashes as we dont have a way to update the models together with updating a release atomically (at any given point if we update the model files, users who install icatcher will fail the hash check until a new version is published)