Unless there is a very good reason too keep it, I would propose to revert commit d67c4f5 in which code specific to the KdV is added to akns_scatter.
That commit makes the behaviour of akns_scatter the same regardless of the PDE. It's semantically correct for NSE as well. It just happens to be a similarity transformation by identity in that case, so that you would be able to cut a corner as long as the code were only for NSE and not for AKNS in general. Without this addition the name should be something like akns_change_of_state to describe adequately what it returns.
Ah, I see. That actually depends on what you expect akns_scatter to do: Solve the AKNS scattering problem or return a scattering matrix of the form [a b' ; b a']. I expect it to do the former, even though with KdV in mind that indeed is inconsistent with the current documentation (which thus should be updated). However, it is consistent with what akns_fscatter actually returns.
The advantage of this approach is that you only have to understand the AKNS system to maintain this function. With the changes, you would also need to understand things specific to KdV to maintain this function. It would reduce the modularity of the code.
Indeed, you could avoid that call by changing akns_scatter_matrix() to akns_change_of_state_matrix(), but then you still need exactly the same calls in akns_scatter_bound_states() since the 1,1-entry of the change of state matrix in the basis you use must be 0 at an eigenvalue in order to use the bidirectional algorithm. That would create an inconsistency within the same file. Furthermore, the KdV and NSE specific transformation matrices need to stay in fnft__akns_discretization.c since only the akns_discretization is available in akns_scatter_bound_states(), so removing it from akns_scatter_matrix() doesn't solve your conceptual point.
All that a maintainer of that file should know is that in general a similarity transformation is needed to find the solution of the AKNS scattering problem, since the assumption just above eq. (3.1) in the AKNS paper does not always hold. That can be clarified with a few extra comments.
What akns_fscatter returns remains different either way. To make it consistent, the polynomials would have to be evaluated in akns_fscatter before return. However, that is not being done for NSE specific reasons, namely because nsev.c needs the polynomials for finding the eigenvalues.
Indeed, you could avoid that call by changing
akns_scatter_matrix()toakns_change_of_state_matrix(), but then you still need exactly the same calls inakns_scatter_bound_states()since the 1,1-entry of the change of state matrix in the basis you use must be 0 at an eigenvalue in order to use the bidirectional algorithm. That would create an inconsistency within the same file. Furthermore, the KdV and NSE specific transformation matrices need to stay infnft__akns_discretization.csince only theakns_discretizationis available inakns_scatter_bound_states(), so removing it fromakns_scatter_matrix()doesn't solve your conceptual point. All that a maintainer of that file should know is that in general a similarity transformation is needed to find the solution of the AKNS scattering problem, since the assumption just above eq. (3.1) in the AKNS paper does not always hold. That can be clarified with a few extra comments.What
akns_fscatterreturns remains different either way. To make it consistent, the polynomials would have to be evaluated inakns_fscatterbefore return. However, that is not being done for NSE specific reasons, namely becausensev.cneeds the polynomials for finding the eigenvalues.Originally posted by @PJ-Prins in https://github.com/FastNFT/FNFT/issues/64#issuecomment-772756290