Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Fix examples after en tree #1827
base: master
Are you sure you want to change the base?
Fix examples after en tree #1827
Changes from 47 commits
3a9c13b
bfd0566
49f72e4
716a2e1
9283f0e
1e427a4
059ab7b
6fef67a
f3b76bb
e2c9cd6
b59adb9
65ae7dd
931d802
2ad2225
ce3313b
df114c1
8875708
5f12cc4
cb7c57c
e768f1f
f60affd
51665d7
dc70b93
f0c7747
45380a5
6170858
7694e2f
009caf6
8ac9eb5
bb9c52d
d0fe08e
10aacd5
1a6a03d
b85f7b7
c19c2ae
4f04499
2cadcbf
2cbf50d
3d76821
dd69d30
dbc3ccf
1310346
599cf2a
856e49b
7d53a24
defedde
013f9f9
42981b1
b7ffbf3
45c6fcf
5615ae1
7a74d26
9e538c7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm not sure how to read this comment. Does this belong in a tutorial?
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.
I guess not.
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.
Should this be surrounded with code blocks
``` ... ```
?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.
Good catch.
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.
I think this code block is missing a closing
```
.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.
Fixed in latest commit.
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.
We should be careful here, because there are two distinct
e0
arguments in the code above, one in eachfun
. I think you are referring to the secondfun
in that code, so it would be good to clarify this, along with some exposition about why there are twofun
s in the first place. (It doesn't have to be super detailed for tutorial purposes, but readers will want some kind of explanation of what is going on here.)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.
Good point. Added some text.
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.
Yes, I definitely think so! Are you planning to resolve this TODO in this PR, or are you leaving this as future work?
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.
It's a good point. The description more or less gets to the limit of what I understand, without going into technical details. Do you have any suggestion of how to improve this section?
We can also just commit the comment and leave it super high level at this point.
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.
Ideally, we would include a more detailed tutorial that goes into the nuances of
spec_refines
proofs involving loops. But writing such a tutorial would take a fair bit of effort, so I can understand if you don't want to do that here. My recommendation would be to: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.
I'm surprised that this isn't adding back more of the
_proofs.v
files that you've changed in this PR, such asarrays_proofs.v
. Any reason not to?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.
I haven't finished the proofs in
arrays_proofs.v
, I only fixed the first one which is the one mentioned in the tutorial.