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

Coersion in strict data types and Omega1(S1). #250

Merged
merged 6 commits into from
Aug 20, 2018
Merged

Conversation

favonia
Copy link
Collaborator

@favonia favonia commented Aug 18, 2018

Close #241.

* correction system, so it is not so bad.
*
* `Up _`: perhaps we can have nghcom? *)
| Ext _ | Univ _ | FHCom _ | V _ | Data _ | Up _ ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need a neutral hcom or a neutral ghcom so that a potential eliminator knows how to deal with it.

auto

let winding-test/50 : Path int int/50 (winding (loopn int/50)) =
auto
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does anyone miss this? I can put it back...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep it, just to make sure we do my introduce some really bad performance degradation. We could even move to different file if you felt it was clutter

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@favonia favonia self-assigned this Aug 18, 2018
@jonsterling
Copy link
Collaborator

@favonia when you run the 50th winding number on your machine, what happens? Travis seems to time out after 10 minute 😦

@favonia
Copy link
Collaborator Author

favonia commented Aug 18, 2018

Glad that we kept the speed test. I apparently screwed up something.

Copy link
Collaborator

@jonsterling jonsterling left a comment

Choose a reason for hiding this comment

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

This PR introduces a very severe performance degradation; I haven't investigated what is happening yet.

the 5th winding number is at least twice as slow (which might not be too terrible), but the 50th cannot even be computed on my computer in a long time.

@jonsterling
Copy link
Collaborator

@favonia Laziness is notoriously difficult to predict 😆 I wonder if some change caused a bad thunk to get forced...

@favonia
Copy link
Collaborator Author

favonia commented Aug 18, 2018

Sorry. Please forget about the above comment (if it still showing in your browser). I probably found the actual cause.

@@ -17,18 +17,35 @@ let loopn (n : int) : Path s1 base base =
| pos n ⇒
elim n [
| zero ⇒ auto
| suc (n ⇒ loopn) ⇒ trans s1 (λ i → loop i) loopn
| suc (n ⇒ loopn) ⇒ trans s1 loopn (λ i → loop i)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This line and the similar line below seem to be causing the trouble.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Heh.. So the issue is that when it associates in one way it is fast, and in the other way infeasible? That's very interesting.

Copy link
Collaborator Author

@favonia favonia Aug 18, 2018

Choose a reason for hiding this comment

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

It somewhat makes sense. I changed the order to make the lemma easier to prove. The new one produces isuc from the outside and is perfect for induction.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we try a harder version of the lemma? I guess analogous to tail recursive function

Copy link
Collaborator

Choose a reason for hiding this comment

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

This performance asymmetry also exists in cubicaltt, btw.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess to prove the "fast" version, it will take some gnarly coercion. Sadly, the goals are tens of thousands of lines long, so it's a little hard to tell what needs to be done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@favonia Can you elaborate on why it makes sense? I guess the performance of coe in fcom depends more on the complexity of the tubes than that of the cap?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ecavallo I am not sure. I have to think harder.

@favonia
Copy link
Collaborator Author

favonia commented Aug 19, 2018

@jonsterling BTW, since we have parameters now, are you treating something like list as strict? I might have treated list s1 wrongly as a strict type.

@favonia
Copy link
Collaborator Author

favonia commented Aug 20, 2018

We decided to give up the speed test for now.

@favonia favonia dismissed jonsterling’s stale review August 20, 2018 14:53

Jon was then brainwashed by @faovonia.

@favonia favonia merged commit 39bbf8f into master Aug 20, 2018
@favonia favonia deleted the coersion-strict branch August 20, 2018 14:54
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.

3 participants