-
Notifications
You must be signed in to change notification settings - Fork 116
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
Porting to HB #733
Porting to HB #733
Conversation
This comment has been minimized.
This comment has been minimized.
799c931
to
f3e3e92
Compare
Nix CI is broken:
|
@CohenCyril is this gitlab CI pipeline a new thing? |
mathcomp/algebra/ssrint.v
Outdated
Notation "m - n" := | ||
(@GRing.add _ (m%N : int) (@GRing.opp _ (n%N : int))) : distn_scope. | ||
(@GRing.add [zmodType of int] (m%N : int) | ||
(@GRing.opp [zmodType of int] (n%N : int))) : distn_scope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to add a only printing
notation with the holes to preserve the printing upon simplification.
@CohenCyril @pi8027 can you check if the recent locking in rat helps this example: 5e84326 |
Here is a squashed and rebase branch: https://github.com/math-comp/math-comp/tree/hierarchy-builder-rebased |
Rebase force pushed, the previous state (before squashing everything) is saved in https://github.com/math-comp/math-comp/tree/hierarchy-builder-before-rebase_2022_05_04 |
So, here is the state of the CI:
|
Out of curiosity, how are compilation times affected? |
Last figure I remember was nearly a factor two but @gares would know better. |
So, here is the state of the CI:
|
A 2x increase in compilation time seems a bit surprising (and maybe worrying) . In particular, the amount of time spent in HB code should be just a small fraction of the compilation time. Isn't that suspicious? What is the main bottleneck here. I think that Coq really needs something like HB, but if users are gonna get a 2x slowdown from it I'm not sure it is ready until it is more optimized. |
The difference is that structures are more regular than before, and less manual tweaks. The time spent in HB commands is negligible, or close to so. The code generated is more likely to trigger bad behavior in conversion heuristics. But so far every time I did investigate a little change here or there would change the cost of a few lines of 100x, so I'm not scared, it is just about spending time on it. |
4c0c3a1
to
d8f1350
Compare
@CohenCyril you can now delete yout hb-ssrnum branch as it is integrated here As a consequence, we had to drop support for Coq 8.13 and 8.14, this now only works with:
|
edc6289
to
df26011
Compare
The 0%O notation doesn't work on them.
In order to remove `0` and `1` (for bottom and top of lattices) that are conflicting with neutral elements of rings.
[HB] Backport #980
Manual merge in master in progress |
This Pull Request is about porting the hierarchy of hand declared algebraic structures to the language of hierarchy-builder.
Rules for contributing to the documentation of this PR:
Tasks:
Progress:
Overlays
finmap.v
to HB finmap#84