ralna / spral

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

something awry in documentation for spral_scaling #217

Open nimgould opened 4 months ago

nimgould commented 4 months ago

I was just looking in the docs for spral scaling, and in particular for the output for the hungarian_scale_sym_example. The input matrix is symmetric, but the output claims that it is unsymmetric (but the values shown are symmetric). Was this output actually generated by the test example ? ... really in docs it always should be!

No big deal, and indeed I just realised that hungarian_scale_unsym and auction_scale_unsym may be exactly what I want to detect dependencies in GALAHAD calculations ... and better than the rather dodgy heuristics I current use! So, good work, team.

jfowkes commented 4 months ago

I would assume not as the example code explicitly passes SPRAL_MATRIX_REAL_SYM_INDEF to the matrix printing utility and that should print out that it is a real symmetric indefinite matrix as you can see from its printing code here: https://github.com/ralna/spral/blob/4a0a72264da773793f552d855b7d65185b421641/src/matrix_util.f90#L396-L398 I guess that it is the output for an earlier run/version of the docs where an unsymmetric example was used.

Note that in #205 we plan to fix these scaling algorithms so that they always return zeros for unmatched as per your suggestion (and in line with what the documentation actually says that they do), but this will not affect the scalings.

nimgould commented 4 months ago

On reading this documentation more carefully, it is clear that the optimization problem posed in the auction and Hungarian sections is incorrect; the objective can't involve the product as stated, since only one of the sigma_i,j in each row/column can be nonzero, and thus the stated product is always zero. Duff & Koster take the product over only nonzero sigma_i,j (and a_i,j), which makes sense, while Hogg & Scott state the incorrect objective but then use a (log) transformed version of Duff & Koster instead (which is what I presume is implemented).

jfowkes commented 4 months ago

Looks like a typo from the paper, we should probably correct the docs to say that the product is over nonzero sigma_i,j.