flintlib / arb

Arb has been merged into FLINT -- use https://github.com/flintlib/flint/ instead
http://arblib.org/
GNU Lesser General Public License v2.1
456 stars 137 forks source link

Fixed bug for aliased mat_mul of window matrices #362

Closed David-Berghaus closed 3 years ago

David-Berghaus commented 3 years ago

I noticed that most mat_mul-functions (with mat_mul_entrywise being an exception) do not work properly for aliased window-matrices. The reason for this is that mat_swap destroys the entanglement of the window matrices with their parent matrix. This can be seen with the following code-snippet:

void arb_mat_mul_aliased_window_test(){
    arb_mat_t A, B, A_window;
    arb_mat_init(A, 2, 2);
    arb_mat_init(B, 2, 2);
    arb_mat_window_init(A_window, A, 0, 0, 2, 2);
    arb_mat_one(A);
    arb_mat_ones(B);

    printf("Before multiplication: \n");
    printf("A: \n");
    arb_mat_printd(A, 10);
    printf("A_window: \n");
    arb_mat_printd(A_window, 10);
    arb_mat_mul_classical(A_window, B, A_window, 53);
    printf("After multiplication: \n");
    printf("A: \n");
    arb_mat_printd(A, 10);
    printf("A_window: \n");
    arb_mat_printd(A_window, 10);

    arb_mat_window_clear(A_window);
    arb_mat_clear(A);
    arb_mat_clear(B);
}

Which produces:

Before multiplication:
A:
[1 +/- 0, 0 +/- 0]
[0 +/- 0, 1 +/- 0]
A_window:
[1 +/- 0, 0 +/- 0]
[0 +/- 0, 1 +/- 0]
After multiplication:
A:
[1 +/- 0, 0 +/- 0]
[0 +/- 0, 1 +/- 0]
A_window:
[1 +/- 0, 1 +/- 0]
[1 +/- 0, 1 +/- 0]

As you can see, A no longer holds the correct result.

My fix performs the pointer-swaps entrywise which makes sure that the corresponding parent object gets updated accordingly. We now have to do O(N^2) pointer-swaps instead of O(1) but the performance impact should be negligible.

fredrik-johansson commented 3 years ago

I agree that this won't lead to a significant performance regression, but I'd like to think some more about possible alternative solutions before merging this. Other methods are affected by this aliasing bug as well.

fredrik-johansson commented 3 years ago

Finally merged in Flint so I will merge this as well. Thanks!