Rem trait for Z Datatypes #488
Conversation
Marvin-Beckmann
left a comment
Marvin-Beckmann
left a comment
There was a problem hiding this comment.
Have you considered to use the modulus as a type with which you can reduce? To me that would feel natural to be possible.
For the modulus we also have a lot of conversions, so maybe there is a native way to do it, and potentially even have some speedups as I suggested. Please check if that is possible somewhere
There was a problem hiding this comment.
please mention that the remainder trait is a standard trait we are missing in the description. This gives the PR more reasoning
| let mut entry = unsafe { self.get_entry_unchecked(i, j) }; | ||
| entry = entry % modulus; | ||
| unsafe { out.set_entry_unchecked(i, j, entry) }; |
There was a problem hiding this comment.
with FLINT you can directly write the result of the modulus operation in the matrix and save yourself one clone for each entry. I would opt for that improvement
There was a problem hiding this comment.
I need to clone the value, as fmpz_poly_scalar_mod_fmpz needs a constant fmpz_poly as a second input, not a mutually one
| assert!(modulus > &1, "Modulus can not be smaller than 2."); | ||
| let num_cols = self.get_num_columns(); | ||
| let num_rows = self.get_num_rows(); | ||
|
|
||
| let mut out = MatZ::new(num_rows, num_cols); | ||
| unsafe { fmpz_mat_scalar_smod(&mut out.matrix, &self.matrix, &modulus.value) }; | ||
|
|
||
| for i in 0..num_rows { | ||
| for j in 0..num_cols { | ||
| let entry = unsafe { out.get_entry_unchecked(i, j) }; | ||
| if entry < 0 { | ||
| unsafe { out.set_entry_unchecked(i, j, entry + modulus) }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| out | ||
| } |
There was a problem hiding this comment.
Please evaluate whether it is faster to do this or:
fmpz_mod_mat_set_fmpz_mat
where you convert the input into a Moduuls.
To me it would also feel quite natural to have an input that would be a modulus
There was a problem hiding this comment.
The method is in FLINT but not in flintsys
Marvin-Beckmann
left a comment
Marvin-Beckmann
left a comment
There was a problem hiding this comment.
make sure to fix the indentation, but looks good and reasonable otherwise.
Description
This PR implements the standard Rem trait for Z, PolyOverZ, MatZ and MatPolyOverZ, which we are currently missing.
Testing
Checklist: