search

gfacciol / mgm

More Global Matching
GNU Affero General Public License v3.0
57 stars 20 forks source link

Results instability with mgm and mgm_multi @ac54122 #2

Closed mcournet closed 5 years ago

mcournet commented 5 years ago

Hello Gabriele,

It seems that two consecutive executions of mgm @ac54122 are not producing the same results. It is the same for mgm_multi @ac54122.

You can refer to the example given in the mgm_multi_ac54122.tar.gz attached. On this example, on two consecutive executions of mgm @ac54122, the following statistics were obtained on these two outputs difference : min=-0.33827555, max=0.72447157, mean=5.896214e-06, std=0.0023109724 (stats obtained on "rectified_disp1.tif - rectified_disp2.tif" also included in the tar.gz).

mgm @ac54122 and mgm_multi @ac54122 are the mgm and mgm_multi default versions of s2p since 29 Oct 2018, as agreed.

Could you please give me a feedback on this topic ?

Thanks, Myriam mgm_multi_ac54122.tar.gz

gfacciol commented 5 years ago

This is due to parallelism. A register is being updated without verifying concurrence here: https://github.com/gfacciol/mgm/blob/a5322ab74ea391658da1ae8d5f51d4c6b4847ae1/dvec.cc#L114 If you call mgm with OMP_NUM_THREADS=1 the issue goes away. Otherwise uncommenting the line above will reduce the difference to numerical precision. However, the result will never be exact due to non commutativity of floating point additions.

gfacciol commented 5 years ago

Just updated the code by removing the comment in: https://github.com/gfacciol/mgm/commit/468b4909dbd1cd9be96f2329e351927306acce73

mcournet commented 5 years ago

Gabriele, thank you very much for your feedback and for the code update. Could you please replace the S2P default version of mgm_multi by https://github.com/gfacciol/mgm/commit/468b4909dbd1cd9be96f2329e351927306acce73 ?

gfacciol commented 5 years ago

done!

On Mon, Feb 11, 2019 at 5:41 PM mcournet notifications@github.com wrote:

Gabriele, thank you very much for your feedback and for the code update. Could you please replace the S2P https://github.com/MISS3D/s2p/tree/master/3rdparty default version of mgm_multi by 468b490 https://github.com/gfacciol/mgm/commit/468b4909dbd1cd9be96f2329e351927306acce73 ?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/gfacciol/mgm/issues/2#issuecomment-462398783, or mute the thread https://github.com/notifications/unsubscribe-auth/AExUaupFq0yPLSHzHbxnKG9pgNRadb92ks5vMZ00gaJpZM4au9jM .