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

Equation Explorer shows Equation4270 wrong #480

Closed
lyphyser opened this issue Oct 10, 2024 · 8 comments · Fixed by #491
Closed

Equation Explorer shows Equation4270 wrong #480

lyphyser opened this issue Oct 10, 2024 · 8 comments · Fixed by #491
Assignees

Comments

@lyphyser
Copy link
Contributor

It says: "Equation4270[x ◇ (x ◇ x) = x ◇ (y ◇ z)]"

But the equation actually is:
equation 4270 := x ◇ (x ◇ x) = x ◇ (y ◇ y)

The final y has been incorrectly replaced with a z.

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

It looks like all may be broken. I will take a look to see what's going on soon.

If someone wants to claim+fix this before I get to it plaese do.

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

equation 953 has been deleted from deleted from AllEquations:

equation 951 := x = y ◇ ((z ◇ x) ◇ (z ◇ x))
equation 952 := x = y ◇ ((z ◇ x) ◇ (z ◇ y))
equation 954 := x = y ◇ ((z ◇ x) ◇ (z ◇ w))
equation 955 := x = y ◇ ((z ◇ x) ◇ (w ◇ x))

I'll bring it back from the dead.

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

claim

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

propose PR #481

@goens
Copy link
Contributor

goens commented Oct 10, 2024

equation 953 didn't disappear, I moved it to Equations.lean in #460

It's still right there where it should:

equation 953 := x = y ◇ ((z ◇ x) ◇ (z ◇ z))

It shouldn't be moved again, because it's in Chapter 2 of the blueprint (see also the corresponding Zulip thread https://leanprover.zulipchat.com/#narrow/stream/458659-Equational/topic/Equations.2Elean.20vs.20Chapter.202)

Maybe the script that breaks is not using Equations.lean? (The lean AllEquations file does import it, but if an external script ignores this it might still break)

Also note that this conflicts with #472 and would probably be fixed by that anyway

@goens
Copy link
Contributor

goens commented Oct 10, 2024

I did just notice that #481 only adds it back as a comment. I guess that works (it's a bit hacky), but we should probably coordinate it with #472 anyway (it's going to conflict)

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

Yeah, but every other equation that has been removed from AllEquations.lean was removed in this way. I agree it's hacky but it's how things have been done in all the prior cases. Long term plan is to load from a .json file for all scripts that need equation data, that should be done this week I think.

I closed my prior PR because it was put back in the split of AllEquations and have now opened a new PR that'll raise an error if this happens again.

@carlini
Copy link
Contributor

carlini commented Oct 10, 2024

propose PR #491

pitmonticone pushed a commit that referenced this issue Oct 10, 2024
Bug #480 was caused by an equation being removed from AllEquations.lean.
This PR makes something like that cause an assert failure which should
make that more obvious.

Closes #480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants