Wrap Calcium fields/numbers #1077
Conversation
Co-authored-by: thofma <thofma@gmail.com>
|
Manual reference counting seems to be working. |
|
Any ideas why [matching: [Oscar#master,Hecke#master,Singular#master] - ubuntu-latest, julia ~1.6.0-0] is stuck in "Run tests" for two hours with no output at all? |
|
I have restarted it. |
|
@thofma Do you have a workaround for JuliaDocs/Documenter.jl#558 ? I want to document e.g. (R::AcbField)(a::ca; parts::Bool=false). Or should I make a named function for this? |
|
No, there is no work around as far as I know. We usually handcraft the "constructor" section like here: https://nemocas.github.io/AbstractAlgebra.jl/latest/polynomial_rings/#Constructors. |
|
This is ready for review! |
|
The rand function is a placeholder, by the way. I can open a separate issue to discuss what kind of options we want here. |
src/calcium/ca.jl
Outdated
|
|
||
| Return the imaginary unit $i$ as an element of *C*. | ||
| """ | ||
| function const_i(C::CalciumField) |
There was a problem hiding this comment.
For AcbField this is called onei.
src/calcium/CalciumTypes.jl
Outdated
| Nothing, (Ref{ca}, Ref{CalciumField}), a, a.parent) | ||
| a.parent.refcount -= 1 | ||
| if a.parent.refcount == 0 | ||
| ccall((:ca_ctx_clear, libcalcium), Nothing, (Ref{CalciumField},), a.parent) |
There was a problem hiding this comment.
Codecov seems to indicate ca_ctx_clear is never called. Could it be the refcounting is not working?
There was a problem hiding this comment.
Yeah, this is precisely the path that only gets called if Julia calls finalizers out of order, which only happens on some versions and on some machines. However, introducing a helper function should solve the coverage issue.
src/calcium/ca.jl
Outdated
|
|
||
| elem_type(::Type{CalciumField}) = ca | ||
|
|
||
| base_ring(a::CalciumField) = Union{} # ? |
There was a problem hiding this comment.
I think these are correct. No need for the ?
There was a problem hiding this comment.
Yeah, I copied this from another module and I was just not sure about the meaning.
src/calcium/ca.jl
Outdated
| # | ||
| ############################################################################### | ||
|
|
||
| export ca, CalciumField, infinity, unsigned_infinity, undefined, unknown, |
There was a problem hiding this comment.
Alphabetical order please. I know there are no other good examples of this.
| # | ||
| ############################################################################### | ||
|
|
||
| # todo: implement nontrivial hash functions on C |
There was a problem hiding this comment.
Does this await something on the C side?
There was a problem hiding this comment.
Yes, it's a nontrivial function to implement (well).
| return r | ||
| end | ||
|
|
||
| function +(a::ca, b::fmpz) |
There was a problem hiding this comment.
Do we have generic code that switches the order of arguments if one ad hoc operation is available?
There was a problem hiding this comment.
No idea what's happening behind the scenes, but the reverse operation works and is tested. I will add the reverse functions anyway to make sure it's fast.
| check_special(r) | ||
| return r | ||
| end | ||
|
|
There was a problem hiding this comment.
Could you separate into sections Ad hoc division, Powering, Unary operations, for conformity with the other files.
src/calcium/ca.jl
Outdated
| return r | ||
| end | ||
|
|
||
| ############################################################################### |
There was a problem hiding this comment.
We usually put the predicates before the arithmetic ops, usually in a section called Basic manipulation.
| end | ||
|
|
||
| function mul!(z::ca, x::ca, y::ca) | ||
| if z.parent != x.parent || x.parent != y.parent |
There was a problem hiding this comment.
We do not check parents in unsafe operators.
There was a problem hiding this comment.
Nonetheless, this check did help catch a bug. I'd rather keep it until the code is more battle-tested.
| @@ -770,7 +782,7 @@ end | |||
| Return the extension of the real sign function taking the value 1 | |||
| strictly in the right half plane, -1 strictly in the left half plane, | |||
| and the sign of the imaginary part when on the imaginary axis. | |||
| Equivalently, $\csgn(x) = x / \sqrt{x^2}$ except that the value is 0 | |||
| Equivalently, $\operatorname{csgn}(x) = x / \sqrt{x^2}$ except that the value is 0 | |||
There was a problem hiding this comment.
is \operatorname standard latex?
There was a problem hiding this comment.
Yes, it's the usual way to define a named operator.
test/calcium/ca-test.jl
Outdated
|
|
||
| end | ||
|
|
||
| @testset "ca.adhoc-operations" begin |
|
Looks good overall. Just a few niggles. |
It's just more convenient to type. I will fix this for consistency. |
Follows #1070.
This adds CalciumField and the ca type.
Work in progress; has most of the functionality (sans ca_poly and ca_mat) but needs docs and tests.