You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
So the two doctests I "fixed" earlier weren't wrong -- they're dependent on the architecture, at least. Here are the issues, and then I'll say a short bit about what I understand wrt the actual problem:
number_field.py:
There's a doctest in here that takes an ideal, factors it, picks a uniformizer for
each factor, and checks to see if the ideal generated by the uniformizer is equal
to the original ideal. The issue is that the uniformizer we produce is different
on different machines. I'm not sure if this is a wholly 32-bit vs. 64-bit issue,
or something else. To be safe, I've removed the doctest, and replaced it with
something much more innocuous.
Someone much braver than I could put the doctest back, with the two possible
results as # 32-bit and # 64-bit. I've fought long enough with this, though, that
I just don't want to see it again -- I'm happy to be overruled on this. If so,
the two doctest results should be:
sage: [ Plist[i] == K.ideal(pilist[i]) for i in range(len(Plist)) ]
[True, False, False] # 32-bit
[True, False, True] # 64-bit
It wouldn't be unreasonable to do this if it passes testall this way on the
whole sage build farm, but my home machine and sage.math isn't enough data for
me to commit to this doctest.
number_field_ideal.py, line 868
This one is stumping me but good. Looking at I.prime_factors() just calls I.factor() ... the difference pops up in the K.ideal(...) call. I know that
the same input is getting passed there on both machines, but one (my machine)
gets (w) back, and the other (sage.math) gets (-w) back. The problem isn't
anywhere in the K.factor() method, though -- it's when you try to print the
result that things go awry! Man, that's frustrating. So I've switched it for
another doctest that doesn't cause trouble, because I know the goal is to get
the release out the door. If someone wants to put it back, here's the original:
Okay, so now for what little I understand about what's going on. The issue with the uniformizers is easier to believe, at least -- pari is producing different results for idealhnf(...) on the two architectures:
I don't know enough about what's happening inside to know what's making it get one result or the other, but the point is that for uniformizers, that difference is the first one that pops up, and it tracks through all the way to ending up with different uniformizers.
Now, for the other problem -- with -w vs. w -- I believe, but I'm not 100% sure, that pari's hnf causes the 1 vs. -1 issue. I know William just got a patch from Karim Belabas to fix exactly this kind of thing, but I'm not completely sure where the difference is cropping up to see if it's the same thing happening. Maybe the patch "over-corrects"?
I'm off to sleep now, but I'll nose around first thing in the morning to see if I can do anything else.
I agree entirely with the above analysis, except that I don't think it's a 32/64 problem (my machine is 32 bit but gave the generator -w).
I give the patch a positive review now so 2.10.2 can hit the streets. For the future, doctest writers should remember to be careful when there's more than one mathematically correct output and algorithms are not always deterministic.
So the two doctests I "fixed" earlier weren't wrong -- they're dependent on the architecture, at least. Here are the issues, and then I'll say a short bit about what I understand wrt the actual problem:
number_field.py
:There's a doctest in here that takes an ideal, factors it, picks a uniformizer for
each factor, and checks to see if the ideal generated by the uniformizer is equal
to the original ideal. The issue is that the uniformizer we produce is different
on different machines. I'm not sure if this is a wholly 32-bit vs. 64-bit issue,
or something else. To be safe, I've removed the doctest, and replaced it with
something much more innocuous.
Someone much braver than I could put the doctest back, with the two possible
results as # 32-bit and # 64-bit. I've fought long enough with this, though, that
I just don't want to see it again -- I'm happy to be overruled on this. If so,
the two doctest results should be:
It wouldn't be unreasonable to do this if it passes testall this way on the
whole sage build farm, but my home machine and sage.math isn't enough data for
me to commit to this doctest.
number_field_ideal.py
, line 868This one is stumping me but good. Looking at
I.prime_factors()
just callsI.factor()
... the difference pops up in theK.ideal(...)
call. I know thatthe same input is getting passed there on both machines, but one (my machine)
gets
(w)
back, and the other (sage.math) gets(-w)
back. The problem isn'tanywhere in the K.factor() method, though -- it's when you try to print the
result that things go awry! Man, that's frustrating. So I've switched it for
another doctest that doesn't cause trouble, because I know the goal is to get
the release out the door. If someone wants to put it back, here's the original:
Okay, so now for what little I understand about what's going on. The issue with the uniformizers is easier to believe, at least -- pari is producing different results for idealhnf(...) on the two architectures:
32-bit:
64-bit:
I don't know enough about what's happening inside to know what's making it get one result or the other, but the point is that for uniformizers, that difference is the first one that pops up, and it tracks through all the way to ending up with different uniformizers.
Now, for the other problem -- with
-w
vs.w
-- I believe, but I'm not 100% sure, that pari's hnf causes the 1 vs. -1 issue. I know William just got a patch from Karim Belabas to fix exactly this kind of thing, but I'm not completely sure where the difference is cropping up to see if it's the same thing happening. Maybe the patch "over-corrects"?I'm off to sleep now, but I'll nose around first thing in the morning to see if I can do anything else.
Component: number theory
Issue created by migration from https://trac.sagemath.org/ticket/2257
The text was updated successfully, but these errors were encountered: