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

Add metamath-knife to automated checks #3279

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

jkingdon
Copy link
Contributor

@jkingdon jkingdon commented Jul 4, 2023

This is mostly for the usage check, but while we're here run a verify too.

The result is that metamath-knife runs and gives the same result I'm seeing locally. Which I think means that metamath/metamath-knife#118 has a bug, or exposes a bug which was there, or something. Once we can figure out what is going on there and fix it, we should be able to get passing tests on this pull request.

Fixes #3250.

This is mostly for the usage check, but while we're here run a verify
too.
@tirix
Copy link
Contributor

tirix commented Jul 4, 2023

That's interesting!
I've created a PR to fix this specific issue, but this will not help in the case of definitional checks.

@tirix
Copy link
Contributor

tirix commented Jul 4, 2023

The re-run passed with the new version of metamath-knife, and I've logged an issue in metamath-knife to track the strange behaviour.

@jkingdon
Copy link
Contributor Author

jkingdon commented Jul 4, 2023

I've logged an issue in metamath-knife to track the strange behaviour.

I saw different strange behavior with iset.mm but apparently the same fix dealt with it (with respect to --verify-usage anyway: as you point out, the problem may still be there for the definitional checks).

Copy link
Contributor

@tirix tirix left a comment

Choose a reason for hiding this comment

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

Three remarks:

  • this check covers the same thing, with the same implementation, as smm3 (smetamath-rs), so the smm3 check could/should be removed.
  • metamath-knife includes a discouraged file generation which is waaay faster than metamath-exe's. It would ease github's server loads and accelerate our CI to move it.
  • metamath-knife has also -t and -m options, for typesetting and markup checks. They are also fast, and cover a bit more than other checkers. You could add it to the -v of this PR.

@jkingdon
Copy link
Contributor Author

jkingdon commented Jul 6, 2023

I guess I'll merge this now and leave the door open for future pull requests for those other issues. They all sound reasonable to me (not that I really know enough about metamath-knife to know whether any of them would be controversial or present any difficulties). But I don't see any particular need to do everything at once.

@jkingdon jkingdon merged commit 7b4b5c4 into metamath:develop Jul 6, 2023
@jkingdon jkingdon deleted the check-usage branch July 6, 2023 03:50
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.

Proposal: be able to declare forbidden axioms for a proof
2 participants