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

SCCE fix for bug in Jax<0.2.7 #130

Merged
merged 3 commits into from
Dec 22, 2020
Merged

Conversation

alexander-g
Copy link
Contributor

Small fix for a bug in Jax<0.2.7 where jax.lax.take_along_axis gives incorrect results for uint8 indices. Very relevant for semantic segmentation.

Alternatively consider updating Jax

@codecov-io
Copy link

codecov-io commented Dec 19, 2020

Codecov Report

Merging #130 (eb7b1dc) into master (37cd585) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   79.22%   79.26%   +0.04%     
==========================================
  Files         110      110              
  Lines        5135     5145      +10     
==========================================
+ Hits         4068     4078      +10     
  Misses       1067     1067              
Impacted Files Coverage Δ
elegy/losses/sparse_categorical_crossentropy.py 100.00% <100.00%> (ø)
...egy/losses/sparse_categorical_crossentropy_test.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37cd585...eb7b1dc. Read the comment docs.

@cgarciae
Copy link
Collaborator

Hey @alexander-g, thanks!

This is interesting, as a library we don't list jax in our dependencies, for development we currently test with jax==0.2.5.

Currently we don't have a policy regarding support for different versions of jax, but given its API is relatively stable our policy could be to support the latest version of jax and ask users to update when possible.

What do you think? We could just update the dev version with poetry as an alternative fix.

@alexander-g
Copy link
Contributor Author

As you want. I still don't quite understand poetry

@cgarciae
Copy link
Collaborator

Easiest way to update jax with poetry is:

poetry update jax jaxlib

This with update the pyproject.toml and poetry.lock files.

@alexander-g
Copy link
Contributor Author

  • Updated Jax
  • Removed the bugfix
  • Kept the test case

@cgarciae
Copy link
Collaborator

Awesome!

@cgarciae cgarciae merged commit 537bab3 into poets-ai:master Dec 22, 2020
@alexander-g alexander-g deleted the scce_fix branch December 23, 2020 09:16
alexander-g added a commit to alexander-g/elegy that referenced this pull request Dec 27, 2020
commit 3b3d88a
Author: alexander-g <[email protected]>
Date:   Wed Dec 23 17:30:41 2020 +0100

    Module.slice()

commit aa02024
Author: alexander-g <[email protected]>
Date:   Wed Dec 23 16:13:29 2020 +0100

    fixing poetry

commit f57d41b
Merge: 250e783 78777ff
Author: alexander-g <[email protected]>
Date:   Wed Dec 23 16:08:14 2020 +0100

    Merge branch 'master' into _slicing

commit 78777ff
Author: Cristian Garcia <[email protected]>
Date:   Tue Dec 22 14:27:11 2020 -0500

    Adds gitpod support to be able to develop elegy on the cloud (poets-ai#134)

    * clear unused docker stuff + add gitpod

    * install on init

    * update gitpod

    * add gitpod to contributing

commit 537bab3
Author: alexander-g <[email protected]>
Date:   Tue Dec 22 18:32:11 2020 +0100

    SCCE fix for bug in Jax<0.2.7 (poets-ai#130)

    * fix for a bug in jax<0.2.7

    * test case +black

    * jax version update

commit 250e783
Merge: 15943e3 c7606f9
Author: alexander-g <[email protected]>
Date:   Fri Dec 4 11:16:24 2020 +0100

    Merge branch 'master' into slicing_modules

commit 15943e3
Author: alexander-g <[email protected]>
Date:   Fri Dec 4 11:01:07 2020 +0100

    black

commit 41fa916
Author: alexander-g <[email protected]>
Date:   Fri Dec 4 10:58:40 2020 +0100

    Docs for add_summary

commit a75c7e3
Author: alexander-g <[email protected]>
Date:   Fri Dec 4 10:41:34 2020 +0100

    Slicing with multi-input modules between start_module and end_module now possible

commit 2f18a9a
Author: alexander-g <[email protected]>
Date:   Wed Dec 2 17:34:09 2020 +0100

    resnet50 fix

commit c525614
Author: alexander-g <[email protected]>
Date:   Wed Nov 25 12:54:15 2020 +0100

    refactoring

commit c4ec83d
Author: alexander-g <[email protected]>
Date:   Wed Nov 25 11:35:52 2020 +0100

    exception handling

commit 0ffc36f
Author: alexander-g <[email protected]>
Date:   Wed Nov 25 10:53:29 2020 +0100

    sliced module is now retrainable

commit 83d2c0c
Author: alexander-g <[email protected]>
Date:   Wed Nov 25 08:34:53 2020 +0100

    added networkx dependency

commit e622fde
Author: alexander-g <[email protected]>
Date:   Mon Nov 23 17:20:17 2020 +0100

    experimental module slicing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants