Open ManonMarchand opened 6 months ago
How does the conversion from python to rust type work? Could this issue be solved on the Rust side?
It silently accepts negative integers from python and converts them into unsigned integers in rust https://pyo3.rs/main/conversions/tables (which lead to crazy values) I think it fits in this pyO3 issue but I'm not sure https://github.com/PyO3/pyo3/issues/3226
Blanketing our integers before sending them to the rust side shouldn't affect the performance too much if it's your concern?
My concern was rather about having a single checkpoint. I thought this could be managed on the Rust side, at conversion time, but it seems I'm wrong.
maybe @fxpineau has an idea on how to do this better?
In which cases do we end up with negative HEALPix indices that are passed from Python to Rust?
Given that pyo3 do not produce and error (so far) when converting from a negative integer to an unsigned integer, we may consider at least two opinions:
I personally have a marked preference for the second bullet (check on Python side), possibly isolating and decorating with a check the call to the Rust method to mitigate the 'cons' (no duplication/no risk to forget, i.e. single checkpoint on the Python side).
It was discovered due to the -1
that is used as padding in cdshealpix.neighbors
when a cell has three neighbors instead of four. This generates a set of healpix's cells with spurious -1
among them.
I like FX's idea to define a decorator on the python's side. This way there is only one decorator to maintain and we can apply it to all methods that accept healpix cells (just as we already do with the validate_lonlat
decorator that ensures that all coordinates sent to the rust side are in degrees, in icrs, and are wrapped)
Due to the conversion from python
int
to rustu8
negative indices are not rejected and create false MOCs instead.To solve this: