Adding documentation and cleaning up arings#4441
Conversation
Ring types that cannot be set from a BigReal no longer need to define a boilerplate set_from_BigReal returning false; get_from_BigReal uses SFINAE to detect whether the member exists and falls back to false otherwise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the hand-written per-ring get_from_double overloads with a single template that uses SFINAE to detect whether a ring defines set_from_double, falling back to false otherwise. As a side effect, ARingQQGMP's existing set_from_double (previously only used by mylift) is now also used for from_double, giving an exact double -> QQ conversion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the hand-written per-ring get_from_BigComplex overloads with a single template that uses SFINAE to detect whether a ring defines set_from_BigComplex, falling back to false otherwise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the hand-written per-ring get_from_complex_double overloads with a single template that uses SFINAE to detect whether a ring defines set_from_complex_double, falling back to false otherwise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the hand-written per-ring get_from_Interval overloads with a single template that uses SFINAE to detect whether a ring defines set_from_Interval, falling back to false otherwise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the hand-written ARingCCi overload of get_from_ComplexInterval with a single template that uses SFINAE to detect whether a ring defines set_from_ComplexInterval, falling back to false otherwise. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The three overloads of mylift with second argument ARingRRi (for ARingRR, ARingRRR, and ARingQQ) had identical bodies, differing only in the type of the first ring. Replace them with one function template parameterized on the first ring type.
Use some templates in aring-translate.hpp
|
A couple tests were failing because one of the changes from my PR made it so that ring maps from These tests ensured that applying these ring maps raised errors, and according to the comments that pointed to #473 , the main point of adding the tests was to show that they no longer segfaulted. I'm proposing that instead of just failing, they should actually work, e.g., |
|
there are no ring maps from RR to QQ -- I'm confused |
|
Well, we can lift from RR to QQ. And that's what ring maps currently use when we have large precision (i.e., when we're using mpfr instead of doubles). So currently, before this PR, we have: i1 : f = map(QQ, RR_53)
o1 = map (QQ, RR , {})
53
o1 : RingMap QQ <-- RR
53
i2 : f 1.0
stdio:2:1:(3):[1]: error: cannot map double to ring type
i3 : g = map(QQ, RR_100)
o3 = map (QQ, RR , {})
100
o3 : RingMap QQ <-- RR
100
i4 : g 1.0
o4 = 1
o4 : QQThis PR is proposing that |
|
lifting is not a ring map, that's why we use it when... there's no ring map... |
|
Good question ... it might have been pretty recently. My guess is maybe #4280, which moved the lifting code to the engine. |
|
I think I understand the point, this is |
|
Another example showing that i1 : f = map(ZZ, QQ);
o1 : RingMap ZZ <-- QQ
i2 : f (5/1)
o2 = 5 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
well, that one is 100% wrong. What's ambiguous about the name of the class "RingMap"? |
|
I'm torn on this one a bit. I agree with Paul that there should be a clear specification that explains when map, lift, promote, and substitute should all be used. Of these, substitute is perhaps the one that can violate the most rules, as it is intended to guess the right answer. I think that, with a specification in place, it would be good to go through these back-end maps, lifts, and promotes to see which ones violate the specification. However, I do have a worry that deleting too many of these back-end functions will have unintended consequences (in that these maps were used as a convenient place to store information so that function calls were consistent). I'm thinking that it might make sense to look through these back-end maps, lifts, and promotes in a separate pull request to try to understand the consequences of the changes that Paul is suggesting. Although, I could be convinced otherwise. |
|
Regarding the use of (not well-defined) ring maps for |
Some more templates
Removes the redundant _from_X suffix from ARing conversion methods: set_from_long, set_from_mpz, set_from_mpq, set_from_double, set_from_doubles, set_from_BigReal, set_from_BigReals, set_from_BigComplex, set_from_Interval, set_from_ComplexInterval, and set_from_int (in tower/dpoly). The source type is already encoded in the argument, making the suffix redundant. As a prerequisite, removes set_from_complex_double (CC/CCC/CCi) and set_from_complex_mpfr (CCC), which were duplicate methods with identical implementations to set_from_doubles and set_from_BigReals respectively, differing only in return type (bool vs void). Retains has_set_from_* names for the concept detectors in aring-translate.hpp (e.g. has_set_from_double, has_set_from_mpq), reverting a mechanical has_set_* rename from an earlier draft. The from_ infix is informative since these concepts detect set() overloads for specific source types. Fixes ARingZZ::invert to call fmpz_set directly rather than the ambiguous set(ElementType&, ElementType) call, which becomes ambiguous with set(ElementType&, long) after the rename since ARingZZ::ElementType = fmpz = long. Adds set(ElementType&, int) to ARingRR, ARingRRR, ARingRRi, ARingCC, ARingCCC, ARingCCi, and ARingQQGMP, which all have both long and double set overloads. Without it, set(elem, int_literal) is ambiguous since int converts to both long and double. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The six free template functions get_from_double, get_from_BigReal, get_from_Interval, get_from_complex_double, get_from_BigComplex, and get_from_ComplexInterval are renamed to try_set. The name better describes what they do: attempt R.set(elem, val) and return false if the ring doesn't support that overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rategy) ARingGFM2 stores field elements as table indices (int), not integers. set(elem&, elem) and set(elem&, long) had the same semantics as copy() and fromZZTable() respectively, but after renaming set_from_long to set(long), calls like ring().set(result, 1) resolved to the elem overload (exact match on int), setting a raw index instead of converting the integer through the GF table. Fix: remove set(elem&, elem) from ARingGFM2 so that set() always means "convert from integer". Replace all element-copy uses of ring().set() with ring().copy() throughout the matrix and ring infrastructure: - dmat-lu.hpp, dmat-lu-inplace.hpp: LU decomposition (original fix) - mat-elem-ops.hpp: DMat getEntry, setEntry, and submatrix helpers - mat-arith.hpp: matrix copy/assignment and transpose - mat-util.hpp: matrix concatenation - smat.hpp: sparse matrix lead_row and vec_set_entry - dmat-lu-zzp-flint.hpp: solution extraction - aring-glue.hpp: ConcreteRing::copy() virtual method ConcreteRing::copy() now uses Element b(*R) + R->copy(b.value(), a) instead of R->set(b, a), using b.value() to give copy() an unambiguous ElementType& argument so template deduction resolves to the correct per-ring copy overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Like GFM2, elements are ints, but representing exponents instead of actual integers. We update set() to promote the integer and use copy() in a few unit tests where we want to copy the exponent instead.
Rename set_from_* methods to set
4 participants
Work from the C/C++ refactoring group. This pull request will