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

updating to sbi v0.18.0? #48

Closed
psteinb opened this issue May 12, 2022 · 6 comments
Closed

updating to sbi v0.18.0? #48

psteinb opened this issue May 12, 2022 · 6 comments

Comments

@psteinb
Copy link
Contributor

psteinb commented May 12, 2022

sbi 0.18.0 brought in tons of changes. I was wondering if there are any plans to adopt those? If so, it might be useful to reflect performance changes in the rendered results.

For example, it might be worth considering to make the sbi version an additional field switch, e.g. like the Task currently.

@jan-matthis
Copy link
Contributor

jan-matthis commented May 17, 2022

Currently, there are no plans to adopt sbibm to the new sbi API. A PR would definitely be welcome!

Maintaining backward compatibility across different versions of sbi and a field switch would be a neat feature. However, we are likely to end up with lots of duplicate code and in addition, we'd need to run tests against all different supported versions. To keep things light and more easily maintainable, my tendency would be to only support a single version of sbi. (So, if one wanted to use an older version of sbi, one would need to install an older version of sbibm as well.)

@psteinb
Copy link
Contributor Author

psteinb commented May 19, 2022

I think there are 2 levels of abstracting the sbi version here:

  • cater to it within sbibm through field switches and if-clause hell. I totally agree, that this can cause quite some headaches and boiler plate code. The runtime cost for the tests is also considerable.
  • on the level of the benchmark data

I think fixing a singular sbi version to a singular sbibm is the way forward. But then, the benchmark data needs to honor this, I believe.

@michaeldeistler
Copy link
Contributor

As a user, the pin to v0.17.2 is sometimes quite annoying since it always downgrades my sbi installation to v0.17.2 whenever I install the benchmark.

How about putting the entire algorithms folder into a separate repo? And then simply

from sbibm_algorithms import rej_abc  # instead of from sbibm.algorithms import rej_abc

@michaeldeistler
Copy link
Contributor

michaeldeistler commented Oct 13, 2022

Or one could simply dropping the pin and raise a warning in the run method if the detected sbi version is not 0.17.2

@jan-matthis
Copy link
Contributor

jan-matthis commented Oct 14, 2022

How about installing with --no-deps as a workaround?

@michaeldeistler
Copy link
Contributor

I did not know about this and it does the job, thanks!

It's still not ideal though because one has to install some deps manually (e.g. diffeqtorch). If anyone knows a way to ignore specific dependencies that would be great!

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

No branches or pull requests

3 participants