Attempts at fixing regression for the OFPModules - #6158
Conversation
I had no particular examples in mind. I just thought that recomputing the presentation in cases where the presentation can be directly read off from how the module is given is redundant (costly) and should be avoided. Why this is bad for the subsequent computations in one special example should be carefully examined. The problem seems to be this computation
in this functon
from
|
|
Thank you for taking a look, @wdecker . Indeed, I spotted the same place where it hangs, but did not find a good reason to explain the different behaviors. |
|
Quick update: The problem seems to not arise in positive characteristic. The following code works for me both on master and on this PR: p = next_prime(32024)
kk = GF(p)
Pp, to_Pp = change_base_ring(c->kk(numerator(c))*inv(kk(denominator(c))), P)
Sp, to_Sp = change_base_ring(to_Pp, S)
Mp, to_Mp = change_base_ring(to_Sp, M)
Np, to_Np = change_base_ring(to_Sp, N)
@time hom(Mp, Np)In this case it runs even faster on master. So it might actually be a problem about coefficient swell over |
|
I confirm that on master @HechtiDerLachs example was still running after about 600s CPU (vmem was 3-4 Gbyte). After commenting out the code from #5990 the same example finished in under 40s. Trying to investigate... |
|
Minor update I modified the input by multiplying some of the |
|
Thanks for the update! I tried a similar thing to multiplying by 3, but did probably not wait the full half hour. Thanks for the report. Do I understand correctly that 200s were achieved on this PR in conjunction with multiplication by 3? Then this multiplication is still making things worse compared to simply applying Singular's implementation for a new presentation. |
|
Another brief update: I printed out the values of |
|
A further update: I ran the modified input over a finite field ( Here the revision from PR #5990 leads to faster computation and a smaller result; obviously, over a finite field there is no problem of "coefficient growth". |
|
Here is a small example to show potential problems with computing syzygy modules (which is related to kernel computation): The sizes of the results are wildly different -- I didn't measure computation time. ADDENDUM a more extreme is the following: (somewhat surprisingly) The computation times did not differ greatly despite a significant difference in the sizes of the result. |
|
In the shortcut introduced by #5990 a The problem is that the given presentation is used and not recomputed as before #5990, hence simplifications are not done leading to longer computation times in commands like kernel when using the presentation. So this is dependent on the example, in some cases the recomputation of the presentation is a problem (which is why #5990 was introduced by @wdecker to make computations finish), in other cases the simplification is the problem. Since the current state of the presentation code is returning a correct result (and is faster) it seems to be reasonable to actually keep it, and rather request simplification/recomputation explicitely where needed? I would still fix the check thing. |
In which line? |
|
In |
|
suggestion: You need that anyway internally, this is just about putting it into nice helpers. Some suggestions for syntax: (I'll try to clean up this comment later) |
|
One other aspect: Simplify or not depends actually to a large extent on what you want to do with it afterwards. For example in a Hom computation it is reasonable to simplify. If simplification is expensive then the Hom would most likely be even more. So this cannot only depend on a heuristic on the module. |
I do this PR to investigate #6088 and the aftermath of #6052. It seems that we need to tune the behavior of
presentationand_presentation_gradedfurther. The changes which I disable here, were introduced in #5990 in order for some surfaces code of @wdecker to work. However, in another project with Simon Felten this now led to a mysterious regression, which I do not fully understand.To reproduce our example, do the following:
On the current master I did not see this code finish. With the changes in this PR it takes only about 40s.
I tried to investigate this a bit and suspected that the denominators in the coefficients might be the cause for the difference in runtimes. But eliminating those denominators manually did not solve the problem. It seems that the seemingly redundant production of the presentation of an already presented module is leading to a significantly better choice of a generating set. And by significant I mean the difference between running for 40 seconds or running forever. That is odd to me and I wanted to put this out for other people to have a look and share their insights. @jankoboehm ? @wdecker ?
It would be nice if we could complement this with @wdecker 's example and try to find the compromise which hopefully resolves both.