Addition of sort_adj to sort an rank 1 array in the same order as an input array

[jvdp1]

Addition of sort_adj to sort an rank 1 array in the same order as an input array.
It is basically an extension of the original sort_index procedure, by making the array index intent(inout), instead of intent(out) only.

To be discussed:

• Name of the procedure: currently called sort_adj, but I think another (more appropriate) name should be found
• Name of the arguments in the API, especially of the associated array currently called index: suggestions of a more appropriate name?
• Is the current implementation of sort_index still valid/appropriate/useful?

@jalvesz sort_adj could replace your internal sorting procedure in your sparse implementation. If not, I think it is still a nice addition to stdlib.

[jalvesz]

What about naming the secondary array array_adjoint or adjoint_array to be explicit about its role?

[perazz]

Thank you for this PR @jvdp1. I agree with @jalvesz and like the adjoint_array name.

Regarding your questions:

• I believe the original sort_index is appropriate because it represents an argsort, that is a widely used operation. So I think that the former API would better be maintained (just call sort_adj with [(j,j=1,n)] input for the list). No idea why the former name was chosen, but I think now it would be late to change it.
• In my library, this function is named sort_and_list, I agree it's not a great name. A probably better option could be sort_pair, as in other languages you can do it with a custom comparison function.

These arguments:

stdlib/src/stdlib_sorting_sort_adj.fypp

Lines 98 to 99 in 28b6f71

	${t1}$, intent(inout) :: array(0:)
	${ti}$, intent(inout) :: index(0:)

could become

 ${t1}$, intent(inout) :: sorted_array(0:) ${ti}$, intent(inout) :: adjoint_list(0:) 

to signal that sorting is performed based on the first argument.

[jvdp1]

Thank you @jalvesz and @perazz for the suggestions.
Based on them I called the new procedure sort_adjoint and the associated array adjoint_array. I kept the name array for the sorted array, as it is used for the other procedures.

sort_index just calls sort_adjoint with an initialised index array.

[perazz]

This is a great example @jvdp1. Could it be made into a test as well? The test would check that array(i)>=array(i-1), and that nint(adjoint(i),kind=${ik}$)==array(i)

[jvdp1]

Good suggestion. I added the tests "test_real_sort_adjointes_" based on it.

[perazz]

LGTM. Pending to add one test program, I think this is ready to merge.

[jalvesz]

@perazz @jvdp1 for the test program, maybe something like one of the tests I wrote for the sparse matrices could work?:

 subroutine test_coo2ordered(error) !> Error handling type(error_type), allocatable, intent(out) :: error type(COO_sp_type) :: COO integer :: row(12), col(12) real :: data(12) row = [1,1,1,2,2,3,2,2,2,3,3,4] col = [2,3,4,3,4,4,3,4,5,4,5,5] data = 1.0 call from_ijv(COO,row,col,data) call coo2ordered(COO,sort_data=.true.) call check(error, COO%nnz < 12 .and. COO%nnz == 9 ) if (allocated(error)) return call check(error, all(COO%data==[1,1,1,2,2,1,2,1,1]) ) if (allocated(error)) return call check(error, all(COO%index(1,:)==[1,1,1,2,2,2,3,3,4]) ) if (allocated(error)) return call check(error, all(COO%index(2,:)==[2,3,4,3,4,5,4,5,5]) ) if (allocated(error)) return end subroutine 

Applying the sorting to row and having col follow. For the sparse matrices I also remove duplicates after sorting, here I think that step is not required.

[perazz]

@jvdp1 besides some conflicts, this PR seems ready to merge. I don't have permission to resolve them on your branch.

[jvdp1]

@jvdp1 besides some conflicts, this PR seems ready to merge. I don't have permission to resolve them on your branch.

Sorry for my delayed answer/reaction.
With the latest PRs, I solved the conflicts and added tests as suggested by @perazz and @jalvesz . With those, I think it will be ready to be merged (if you agree of course).

[perazz]

LGTM @jvdp1, thank you for resolving the conflicts with the updated codebase.

[jalvesz]

LGTM @jvdp1 I also think this PR is ready to be merged.