Skip to content

[Merged by Bors] - chore: add missing #align statements#1902

Closed
jcommelin wants to merge 5 commits intomasterfrom
missing-mathlib-aligns
Closed

[Merged by Bors] - chore: add missing #align statements#1902
jcommelin wants to merge 5 commits intomasterfrom
missing-mathlib-aligns

Conversation

@jcommelin
Copy link
Copy Markdown
Member

@jcommelin jcommelin commented Jan 28, 2023

This PR is the result of a slight variant on the following "algorithm"

  • take all mathlib 3 names, remove _ and make all uppercase letters into lowercase
  • take all mathlib 4 names, remove _ and make all uppercase letters into lowercase
  • look for matches, and create pairs (original_lean3_name, OriginalLean4Name)
  • for pairs that do not have an align statement:
    • use Lean 4 to lookup the file + position of the Lean 4 name
    • add an #align statement just before the next empty line
  • manually fix some tiny mistakes (e.g., empty lines in proofs might cause the #align statement to have been inserted too early)

Open in Gitpod

Copy link
Copy Markdown
Member

@kbuzzard kbuzzard left a comment

Choose a reason for hiding this comment

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

I have actually looked at all 222 changed files here; I left very few comments on specific additions. I think this is a great PR which presumably will be rotting very quickly; if I had merge powers I'd almost be tempted to merge now and fix up the potential issues I flagged later.

The PR seems to have caught the fact that in four or five files, nothing is aligned at all! Many additions are "null" in the sense that they're aligning xyz with xyz but there are still a huge number of genuine alignments.

I've always been aware that mathlib3port was adapted to deal with to_additive aligns some time after to_additive was ported, so there would be some to_additive stuff which had been missed. The technique used in this PR has caught these, but it has also caught declarations which have been generated by ext, simps and reassoc. It is probably worth opening an issue in mathlib3port asking that these declarations are auto-aligned by the porting program, otherwise you'll have to run this code again and again as the port progresses. Once the porting program knows about these (and it might do so already, I'm just observing that clearly it didn't always know about these things) you might just need to run it one last time.

Excellent work! Let's get this merged quickly.

@fpvandoorn
Copy link
Copy Markdown
Member

fpvandoorn commented Jan 29, 2023

Thanks, this is great!

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jan 29, 2023
bors bot pushed a commit that referenced this pull request Jan 29, 2023
This PR is the result of a slight variant on the following "algorithm"

* take all mathlib 3 names, remove `_` and make all uppercase letters into lowercase
* take all mathlib 4 names, remove `_` and make all uppercase letters into lowercase
* look for matches, and create pairs `(original_lean3_name, OriginalLean4Name)`
* for pairs that do not have an align statement:
  - use Lean 4 to lookup the file + position of the Lean 4 name
  - add an `#align` statement just before the next empty line
* manually fix some tiny mistakes (e.g., empty lines in proofs might cause the `#align` statement to have been inserted too early)
@bors
Copy link
Copy Markdown

bors bot commented Jan 29, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore: add missing #align statements [Merged by Bors] - chore: add missing #align statements Jan 29, 2023
@bors bors bot closed this Jan 29, 2023
@bors bors bot deleted the missing-mathlib-aligns branch January 29, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants