Skip to content

Exclusion of observed variables from posterior#144

Open
vitkl wants to merge 6 commits into
masterfrom
vitkl-obs-exclusion
Open

Exclusion of observed variables from posterior#144
vitkl wants to merge 6 commits into
masterfrom
vitkl-obs-exclusion

Conversation

@vitkl

@vitkl vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor

Add tests verifying that observed variables are excluded.

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

Hi @adamgayoso @jjhong922

Do you know where the JAX error is coming from and how to solve it? Tests fail at scvi-tools import. Thanks!

@adamgayoso

Copy link
Copy Markdown
Contributor

can look into it, pinning scvi-tools will not solve this problem. I would leave it >0.15.0

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

I see. When did you introduce jax?

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

Maybe JAX version needs pinning.

In this test, it is:

Collecting jax>=0.3
  Downloading jax-0.3.7.tar.gz (944 kB)

but in scvi-tools latest successful commit it is:

Collecting jax>=0.3
  Downloading jax-0.3.6.tar.gz (936 kB)

@adamgayoso

Copy link
Copy Markdown
Contributor

0.15.0 is when we added it, it looks like the issue is optax 0.1.2 released 3 days ago, as our CI is using 0.1.1

@adamgayoso

Copy link
Copy Markdown
Contributor

oh weird it could also be jax, looks like 0.3.7 is like half-released? It's not released on github but is on pypi

@adamgayoso

Copy link
Copy Markdown
Contributor

jax-ml/jax#10326

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

I see. I will try to fix jax and jaxlib here - and wait for updates.

Did you consider making jax optional for scvi-tools?

@adamgayoso

Copy link
Copy Markdown
Contributor

We considered making it optional, but feel it will become a larger part of the framework in the future so decided against it. Overtime it will become a more seamless dependency like pytorch I imagine.

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

I would hope so too. Looks like fixing version to earlier jax and jaxlib works.

@adamgayoso

Copy link
Copy Markdown
Contributor

I mean, stuff like this will get sorted out on its own in a few days. A few years ago we all used to have problems with numba, which seems to have been somewhat smoothed out.

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

I see. Thanks @adamgayoso ! I will remove fixed versions before merging this.

@vitkl

vitkl commented Apr 16, 2022

Copy link
Copy Markdown
Contributor Author

WRT exclusion of observed variables:

This appears to be not an issue for standard cell2location - but looks like it is a problem for Messenger AutoGuides: there is no filtering of observed variables in the changes introduces to posterior sampling is this PR scverse/scvi-tools#1267. So filtering needs to be reintroduced (if possible) or that PR reverted.

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.

2 participants