Nemocas / Nemo.jl

Julia bindings for various mathematical libraries (including flint2)
http://nemocas.github.io/Nemo.jl/
Other
176 stars 57 forks source link

Add is_zero_entry method; remove is_zero_row methods #1801

Closed fingolfin closed 1 week ago

fingolfin commented 1 week ago

With the addition of the new is_zero_entry method, the removed is_zero_row methods are not faster than the generic one. Also add some tests, which were also used for benchmarking, together with

@benchmark Nemo.is_zero_row($M,4)
codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.96%. Comparing base (784c28a) to head (4fc5241).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1801 +/- ## ======================================= Coverage 85.95% 85.96% ======================================= Files 95 95 Lines 36417 36398 -19 ======================================= - Hits 31303 31290 -13 + Misses 5114 5108 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lgoettgens commented 1 week ago

Can someone (@fingolfin @thofma) un-require the nightly CI job until https://github.com/oscar-system/Oscar.jl/issues/3882 is resolved?

fingolfin commented 1 week ago

Huh, from my POV the tests against Julia nightly should never be required. Changed it.

fieker commented 1 week ago

is_zero_row should be done for ZZ and ZZ mod, but in a useful way. The removed implementation is highly non-optimal, this can be sped up by a factor of 10 or so

fieker commented 1 week ago

although it is probably non critical, timewise

lgoettgens commented 1 week ago

There is actually a generic function in https://github.com/Nemocas/AbstractAlgebra.jl/blob/762eab1920547d2e4c3f3e77a4976fff119bf206/src/Matrix.jl#L247-L254, that delegates to a Nemo-backed is_zero_entry. Thus Max's removal is essentially no change at all.

fieker commented 1 week ago

the point is that is_zero_row for ZZ should be done via