search

murraylab / PsychRNN

https://psychrnn.readthedocs.io
MIT License
133 stars 42 forks source link

set numpy version in tests #47

Closed plhijk closed 1 year ago

plhijk commented 1 year ago

Recent numpy 1.24 deprecated some apis, breaking compatibilty with TF. So I added a constrain to numpy version in tests. I also removed the wildcard for TF v2, since recent versions seem to introduce some breaking changes.

syncrostone commented 1 year ago

Thank you! Where are you seeing 2. fail for tensor flow? I re-ran the wildcard tests on PR #44 and it seems to be working fine at least with python 3.8 and TF 2.? (currently rerunning py 3.7 and TF 2.*). If possible I'd like to keep the wildcard in because the goal is to keep PsychRNN forward compatible with new versions of Tensorflow.

codecov[bot] commented 1 year ago

Codecov Report

Merging #47 (db06dfd) into master (ea66983) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #47   +/-   ##
=======================================
  Coverage   76.07%   76.07%           
=======================================
  Files          14       14           
  Lines         882      882           
=======================================
  Hits          671      671           
  Misses        211      211

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

plhijk commented 1 year ago

I was just concerned about incompatibility, e.g. TF v2.11 changed tf.keras.optimizers.Optimizer class (see https://github.com/tensorflow/tensorflow/releases/tag/v2.11.0). Not sure how much it affects the implementation in this package. The test with py3.8 and TF2.11 passed so it seems ok.

syncrostone commented 1 year ago

I was just concerned about incompatibility, e.g. TF v2.11 changed tf.keras.optimizers.Optimizer class (see https://github.com/tensorflow/tensorflow/releases/tag/v2.11.0). Not sure how much it affects the implementation in this package. The test with py3.8 and TF2.11 passed so it seems ok.

The default optimizer in psychrnn is tf.compat.v1.train.AdamOptimizer so it should keep working fine (and tests that use it pass) so I'm going to keep the wildcard in. Thanks for the PR!