-
Notifications
You must be signed in to change notification settings - Fork 257
Changed Complex strict equality internally #3874
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
base: development
Are you sure you want to change the base?
Conversation
This looks like a good idea to me. However, should the method name be |
Since it's an internal method I don't mind either way, but for the record I was going with matching |
Having investigated a bit further, I am still concerned that |
What was the example you tried it on? |
@mikestillman and I create some relatively large Koszul complexes to compare |
Ah, I see. Let me explain. What you've observed about the difference between isIdentical and === is simply the fact that === for a HashTable (which like isIdentical checks === on all content) is slower than === for a MutableHashTable (which checks two fixed hash numbers)! In other words, even if you changed Complex into a HashTable, the timing would exactly match isIdentical, no better and no worse! The reason in your examples == and isIdentical took the same amount is that internally == for modules and matrices first checks === before doing anything more complicated, and if you have large identical koszul complexes that === check succeeds immediately, which is exactly what isIdentical does, so they take the same amount. It's just that the matrices are large so checking entry by entry is taking a while. In fact, in all cases simply constructing the koszul complex took an order of magnitude longer than isIdentical, so I'm not sure if the cost of using isIdentical is relevant at all. In other words, isIdentical is not going to be the bottleneck in a computation if constructing the objects in the first place takes an order of magnitude longer. Finally, to respond to your initial concern, == and isIdentical are absolutely not the same when the modules are equal but not identical, which is the case you presumably want to avoid in liftMapAlongQuasiIsomorphism and other places. Here is an example: debug needsPackage "FGLM"
I = cyclic(5)
C = complex module I
D = complex module ideal random I_*
elapsedTime isIdentical(C, D) -- false, .00001999s elapsed
elapsedTime C === D -- false, .000003078s elapsed
elapsedTime C == D -- true, .478898s elapsed If you raise that 5 to a 6 or 7, == should take up to half an hour! |
To recap the conversation during the meeting, the example above shows that this statement about
I spent quite a bit of time with |
This changes how two complexes are compared to be identical internally, without incurring any computational cost. The point is that these basic things didn't work before: (I added these as a test)
R = QQ[x] C = complex R D = complex R id_C | id_D id_C || id_D id_C // id_D
But now complexes are compared like so:
Fixes #3865 and #3868