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

Normalizing proof of commutativity +-comm #149

Closed
wvhulle opened this issue May 31, 2019 · 11 comments
Closed

Normalizing proof of commutativity +-comm #149

wvhulle opened this issue May 31, 2019 · 11 comments

Comments

@wvhulle
Copy link

wvhulle commented May 31, 2019

Normalizing the application of +-comm to cubical numerals fails for relatively low values such as for example in +-comm 4 6. Is this normal expected behaviour? Normalization works for any number when the implementation of the numerals in the standard library is used.

@mortberg
Copy link
Collaborator

That is very strange! There seems to be an exponential blow-up. This does not happen for +-suc, so I think the problem must come from the use of compPath which builds a bunch of hcomp's. The number of them should be linear though, so I would expect this to be linear as well.

@Saizan Thoughts?

@Saizan
Copy link
Contributor

Saizan commented May 31, 2019

I think you are right that the culprit is compPath, it seems more expensive to increase the second argument, which is the one creating more compositions.

Also, using _□_ seems to make things faster regardless.

@Saizan
Copy link
Contributor

Saizan commented Jun 3, 2019

Ok, it's a problem of sharing for partial elements, basically hcomp has to look at the partial elements to decide if they are a constructor and then recurse. However, because of how partial elements are manipulated, the recusive call has to reduce the original partial element from scratch.

The reason for this inefficiency is that there isn't a good way to generate partial elements during reduction at the moment. Currently the only way to produce non-trivial partial elements on the fly is primPOr which is a single disjunction, so things might get a little ugly if reducing an hcomp on an inductive type leads to a term with a bunch of nested primPOr, though it would be more efficient.
Maybe the solution is an n-ary version of primPOr.

For naturals and other builtin types with literals we should also have custom implementations that take advantage of the compact representation available for closed terms. E.g., for Nat, hcomp [phi -> 100] 100 should compute to 100 directly rather than going through 1 + hcomp [phi -> 99] 99.
This is actually orthogonal and would be sufficient for +-comm 4 6.

@mortberg
Copy link
Collaborator

mortberg commented Jun 3, 2019

I noticed a massive speed-up in cubicaltt when computing hcomp as you suggest for closed terms of datatypes. It's not clear to me how we can utilize this in the presence of open terms though... But if we write an efficient closed term evaluator we should definitely use this optimization.

@Saizan
Copy link
Contributor

Saizan commented Jun 3, 2019

In agda's internals, 100 is represented as LitNat _ 100 where LitNat :: Range -> Integer -> Literal, so even in the evaluator for open terms we could have a special case for when all the sides are of the form Literal l.

@mortberg
Copy link
Collaborator

mortberg commented Jun 3, 2019

Won't this force the sides to always be evaluated? I tried this in cubicaltt and it made things extremely slow because we couldn't rely on laziness to avoid unfolding things

@Saizan
Copy link
Contributor

Saizan commented Jun 3, 2019

For open computation we have to reduce the sides to WHNF anyway.

For compositions that are known to be closed we can just use the base, that would be a different optimization.

@mortberg
Copy link
Collaborator

mortberg commented Jun 3, 2019

I see. Please try to optimization then! A good benchmark is: https://github.com/agda/cubical/blob/master/Cubical/Experiments/Problem.agda#L69

It should evaluate to refl at pos 0. I cubicaltt it is instant (with the optimization for hcomp in nat) but in Cubical Agda it never terminated for me.

@Saizan
Copy link
Contributor

Saizan commented Jun 3, 2019

A good benchmark is: https://github.com/agda/cubical/blob/master/Cubical/Experiments/Problem.agda#L69

Might be good, but unless I'm missing something it's not a benchmark involving hcomp on inductive types much. Seems like it's all about HITs and the universe.

@Saizan
Copy link
Contributor

Saizan commented Jul 19, 2019

Optimized hcomp for data types and especially literals in Agda master, agda/agda@fdbe073

Normalizing +-comm 4 6 is much faster, and things scale reasonably to larger numbers (+-comm can't get better than linear in the second argument).

@wvhulle would you like to test it out?

@Saizan
Copy link
Contributor

Saizan commented Aug 15, 2019

I will close this as my testing indicates it's much better now.

@Saizan Saizan closed this as completed Aug 15, 2019
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

No branches or pull requests

3 participants