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

refactor: ArgsPacker #3621

Merged
merged 6 commits into from
Mar 14, 2024
Merged

refactor: ArgsPacker #3621

merged 6 commits into from
Mar 14, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Mar 6, 2024

This introduces the ArgsPacker module and abstraction, to replace the exising PackDomain/PackMutual code. The motivation was that we now have more uses besides Fix.lean (GuessLex and FunInd), and the code was spread in various places.

The goals are

  • consistent function naming withing the the PSigma handling, the PSum handling, and the combined interface
  • avoid taking a type apart just based on the PSigma/PSum nesting, to be robust in case the user happens to be using PSigma/PSum somewhere. Therefore, always pass an arity or numFuncs or varNames around.
  • keep all the PSigma/PSum encoding logic contained within one module (ArgsPacker), and keep that module independent of its users (so no EqnInfos visible here).
  • pick good variable names when matching on a packed argument
  • the unary function now is either called fun1._unary or fun1._mutual, never fun1._unary._mutual.

This file has less heavy dependencies than PackMutual had, so build parallelism is improved as well.

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 6, 2024 15:12 Inactive
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Mar 6, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Mar 6, 2024

Mathlib CI status (docs):

  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f0a762ea4d2a78b1f0e484ffa9154c5f495d4c39 --onto ae492265fec103aa834d897bf9f68c94d10f0785. (2024-03-06 15:17:36)
  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 0072d13bd40aef791090d1e3e964413f1176162e --onto ae492265fec103aa834d897bf9f68c94d10f0785. (2024-03-06 17:23:41)
  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-03-07 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-03-07 08:56:58)
  • 💥 Mathlib branch lean-pr-testing-3621 build failed against this PR. (2024-03-10 09:58:33) View Log
  • 💥 Mathlib branch lean-pr-testing-3621 build failed against this PR. (2024-03-10 10:46:45) View Log
  • ✅ Mathlib branch lean-pr-testing-3621 has successfully built against this PR. (2024-03-10 21:16:48) View Log
  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-03-10 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-03-10 22:34:50)
  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-03-11 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-03-11 12:17:14)
  • 💥 Mathlib branch lean-pr-testing-3621 build failed against this PR. (2024-03-13 14:06:09) View Log
  • 💥 Mathlib branch lean-pr-testing-3621 build failed against this PR. (2024-03-13 15:09:25) View Log
  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 0959bc45d2a50b1dbae8c4cf0b867a03b8fd4ffe --onto 317adf42e92a4dbe07295bc1f0429b61bb8079fe. (2024-03-14 15:01:48)

@nomeata nomeata force-pushed the joachim/argspacker branch from ed1c691 to 23a93fa Compare March 6, 2024 16:31
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 6, 2024 16:39 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 6, 2024 17:18 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 6, 2024 17:35 Inactive
@nomeata
Copy link
Collaborator Author

nomeata commented Mar 6, 2024

This is done, I think. Nice improvements to the variable names shown by termination_by?.
I’ll wait for the next nightly-with-mathlib for some extra testing though. done

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 6, 2024 17:44 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 7, 2024 08:50 Inactive
@nomeata nomeata force-pushed the joachim/argspacker branch from 114e2c7 to ad9a044 Compare March 10, 2024 08:34
@nomeata nomeata changed the base branch from master to nightly-with-mathlib March 10, 2024 08:34
@nomeata nomeata added toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN and removed toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN labels Mar 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 10, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan full-ci labels Mar 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 10, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Mar 10, 2024
This introduces the `ArgsPacker` module and abstraction, to replace the
exising `PackDomain`/`PackMutual` code. The motivation was that we now
have more uses besides `Fix.lean` (`GuessLex` and `FunInd`), and the
code was spread in various places.

The goals are

 * consistent function naming withing the the `PSigma` handling, the
   `PSum` handling, and the combined interface
 * avoid taking a type apart just based on the `PSigma`/`PSum` nesting,
   to be robust in case the user happens to be using `PSigma`/`PSum`
   somewhere. Therefore, always pass an `arity` or `numFuncs` or
   `varNames` around.
 *  keep all the `PSigma`/`PSum` encoding logic contained within one
    module (`ArgsPacker`), and keep that module independent of its users
    (so no `EqnInfos` visible here).
 * pick good variable names when matching on a packed argument
 * the unary function now is either called `fun1._unary` or
   `fun1._mutual`, never `fun1._unary._mutual`.

This file has less heavy dependencies than `PackMutual` had, so build
parallelism is improved as well.
@nomeata nomeata force-pushed the joachim/argspacker branch from ad9a044 to 9857a3b Compare March 10, 2024 21:37
@nomeata nomeata changed the base branch from nightly-with-mathlib to master March 10, 2024 21:37
@nomeata nomeata added the will-merge-soon …unless someone speaks up label Mar 10, 2024
@nomeata nomeata marked this pull request as ready for review March 10, 2024 21:37
@nomeata nomeata requested a review from leodemoura as a code owner March 10, 2024 21:37
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 10, 2024 21:48 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 11, 2024 11:29 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 13, 2024 11:08 Inactive
@nomeata nomeata removed the full-ci label Mar 13, 2024
@nomeata nomeata closed this Mar 13, 2024
@nomeata nomeata reopened this Mar 13, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 13, 2024 13:45 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 13, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan full-ci and removed builds-mathlib CI has verified that Mathlib builds against this PR labels Mar 13, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 13, 2024 14:08 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 13, 2024
@nomeata nomeata enabled auto-merge March 14, 2024 13:28
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 14, 2024 14:00 Inactive
@nomeata nomeata removed the full-ci label Mar 14, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 14, 2024 14:47 Inactive
@nomeata nomeata added this pull request to the merge queue Mar 14, 2024
Merged via the queue into master with commit f89ed40 Mar 14, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN will-merge-soon …unless someone speaks up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants