ralna / spral

Sparse Parallel Robust Algorithms Library
https://ralna.github.io/spral/
Other
102 stars 27 forks source link

Dimension of argument `match` for matching-based scaling methods is unclear for non-square matrices #207

Closed mjacobse closed 1 month ago

mjacobse commented 1 month ago

Documentation says match(n) (auction_scale_unsym, hungarian_scale_unsym), code says match(m) (auction_scale_unsym_int32, auction_scale_unsym_int64, hungarian_scale_unsym_int32, hungarian_scale_unsym_int64).

The tests pass an oversized match(maxn) with m, n <= maxn and the examples are using a square matrix, so they do not really provide clarification either.

Indeed, neither match(m) nor match(n) seem to work correctly in all cases. For both there are settings with either m < n or m > n that lead to memory access violations or uninitialized returned values.

jfowkes commented 1 month ago

Thanks @mjacobse, what a mess. It's not clear to me what the correct setting should be here.

jfowkes commented 1 month ago

@nimgould any ideas on what was intended here?

jfowkes commented 1 month ago

@nimgould says that the documentation is incorrect and the code is correct so it should be match(m) in the docs.

mjacobse commented 1 month ago

Yeah that makes sense. On further inspection, the only memory issues I can find when using m are with singular matrices and special cases of #200, so we can discuss and handle those there.