-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adding consistency tests & fixing data issues #187
Comments
This comment was marked as outdated.
This comment was marked as outdated.
The So we are in a good state. We could add this as test, just modified to skip tables starting with |
Yes, exactly.
We import all the tables anyway so adding this should be easy. How long did it take to finish on your machine? |
Two minutes for this: @time for nam in genchartab()
# skip tables containing only unipotent character types
startswith(nam, "uni") && continue
# workaround issue #117
startswith(nam, "2F4") && continue
startswith(nam, "Ree") && continue
println(nam)
g = genchartab(nam)
o = sum(nrchars(c)*degree(c)^2 for c in g)
if o != order(g)
println(" expected: ", order(g))
println(" actual: ", o)
end
end By far the slowest is |
Should we skip |
Sure. But we should add a comment saying why we skip it. I just had a look, it seems this is by far the largest table, the file is ~1.6 MB (next largest is
Seems most of this is for simplification: if I change
by switching the default value to
So on the one hand we probably should switch to JSON eventually. But on the other hand, I really wonder if we can't optimize the simplification rules. For example, if there is a single summand and the numerator or denominator is It also seems pointless to re-compute And then it turns out function Base.hash(a::FracElem, h::UInt)
b = 0x8a30b0d963237dd5%UInt
# We canonicalise before hashing
return xor(b, hash(numerator(a, true), h), hash(denominator(a, true), h), h)
end and we have
But the majority work is spent in |
Ok, but this is done already in the tests. So computing the order is just 12 seconds. |
I've added the group order test in #196 and it seems to be fast enough. |
Inspired by #186 (comment) I thought this would be a good thing for a consistency test...
I think we should add this and other such tests somewhere. Ideally so that one can run them once, now, and fix issues; or run them on new tables as we add them. If they are fast enough we can also run them in PR CI tests, or at least in a daily cron job.
The first test inspired by the above, is this:
It ends up reporting a few inconsistencies:
The text was updated successfully, but these errors were encountered: