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

Refactor according to v2 design doc #17

Merged
merged 22 commits into from
Feb 22, 2019
Merged

Refactor according to v2 design doc #17

merged 22 commits into from
Feb 22, 2019

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Feb 20, 2019

Resolves #16
See design doc for details.

Summary

  • Support tensor-valued dims and multivariate distributions.
  • Change constructor delegation order following Refactor constructor delegation order #13:
  • Simplify FunsorMeta.__call__() by replacing effectful with a single settable function, defined by funsor.interpreter. The function is accessed by interpreter.interpret() and funsor.eval() is renamed to interpreter.reinterpret().
  • Implement three interpretations by default: eager, lazy (for lazy ops but eager substitution), and reflect (completely lazy).
  • Delete everything not essential to basic design goals: bracket indexing, Align, Branch, simplification, obsolete engines, pattern matching, ...

Tasks

  • add funsor.domains
  • add funsor.interpretations
  • update funsor.terms
  • update funsor.torch
  • delete funsor.contract etc.
  • update examples (mark all as xfail)
  • remove funsor.adjoint tests
  • update test/test_terms.py
  • delete obsolete engines (contract_engine, tree_engine)
  • update test/test_torch.py
  • update funsor.distributions
  • update test/test_distributions.py

@fritzo fritzo requested a review from eb8680 February 20, 2019 20:18
@fritzo fritzo changed the title Rewrite funsor.terms according to v2 design doc Refactor according to v2 design doc Feb 20, 2019
@fritzo fritzo marked this pull request as ready for review February 21, 2019 02:03
@fritzo
Copy link
Member Author

fritzo commented Feb 22, 2019

@eb8680 I'm ready to merge this any time.

  • test_terms.py passes tests, but could use more tests
  • test_torch.py passes Tensor tests, but needs Function tests
  • distributions is broken, but I can get to that later
  • i have oversimplified the interpretation stuff into a pathetic mess. sorry. you will need to rewrite it before you can use it for adjoints or optimization.
  • i have left minipyro and examples in the repo. they are broken. feel free to delete them and we can add them back in a more principled manner.

@fritzo
Copy link
Member Author

fritzo commented Feb 22, 2019

BTW please do a "squash merge" to hide my missteps 😉

Copy link
Member

@eb8680 eb8680 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much cleaner! I'm happy with the state of things now so I think it makes sense to merge this and address remaining things in self-contained followup issues/PRs.

@eb8680 eb8680 merged commit b2c980b into master Feb 22, 2019
@fritzo fritzo mentioned this pull request Feb 23, 2019
7 tasks
@fritzo fritzo deleted the v2 branch March 7, 2019 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants