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

FR: adding conditional OT neural solvers #419

Open
cspollard opened this issue Aug 28, 2023 · 6 comments
Open

FR: adding conditional OT neural solvers #419

cspollard opened this issue Aug 28, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@cspollard
Copy link

First off: I really like this library. I was able to get it running rather quickly and test on a few toy low-dimensional examples that close nicely using the provided neural solvers.

The main reason I can't switch to using it "wholesale" is the lack of support of conditional OT solvers along the lines of

https://arxiv.org/abs/2206.14262.

Is that on the road map? Or is it implemented somewhere that I just haven't found yet?

@michalk8
Copy link
Collaborator

Hi @cspollard , thanks a lot for the interest! I think this would be a really nice contributions and afaik @MUCDK is/was working on an implementation of PICNNs.

@michalk8 michalk8 added the enhancement New feature or request label Aug 28, 2023
@MUCDK
Copy link
Contributor

MUCDK commented Aug 28, 2023

Hi @cspollard and @michalk8 ,

We have an implementation in moscot (theislab/moscot#567), including all output classes etc. I think it's up to you @michalk8 and @marcocuturi whether you would like to have this in OTT.

If we implement it, I think we should refactor a bit on the OTT-side, i.e. introduce more abstract classes for the output/cond. output classes, especially given that it's not unlikely we'll add another neural solver to OTT soon.

We could use our moscot implementation as a baseline and start from there.

@cspollard
Copy link
Author

Thanks for the quick feedback/action!

I think you'd find a number of users from collider physics interested in this. So far we have rolled our own implementations of the PICNNs, but this is nontrivial to get right.

@marcocuturi
Copy link
Contributor

Hi @cspollard and thanks for your interest in OTT!

I think it's a good idea to add PICNNs to OTT. @MUCDK @michalk8 we should discuss this!

@jannisborn
Copy link

@cspollard @michalk8 what's the state on this issue? We'd also be interested to see PICNNs supported in ott-jax. Could you describe what would be required beyond building a PICNN child class of ICNNs? The initialization scheme proposed in the CondOT paper seems to be already available.

@cspollard
Copy link
Author

Hi @jannisborn. I’m not sure. From previous messages I understood this was not a big task. If it requires a lot of development, maybe we could “chip in” to help. I let @marcocuturi @michalk8 @MUCDK comment.

Chris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants