-
Notifications
You must be signed in to change notification settings - Fork 197
Addition of sort_adj to sort an rank 1 array in the same order as an input array
#849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
What about naming the secondary array |
|
Thank you for this PR @jvdp1. I agree with @jalvesz and like the Regarding your questions:
These arguments: stdlib/src/stdlib_sorting_sort_adj.fypp Lines 98 to 99 in 28b6f71
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. |
| integer, allocatable :: array(:) | ||
| real, allocatable :: adjoint(:) | ||
|
|
||
| array = [5, 4, 3, 1, 10, 4, 9] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion. I added the tests "test_real_sort_adjointes_" based on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Pending to add one test program, I think this is ready to merge.
|
@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 |
Co-authored-by: Federico Perini <federico.perini@gmail.com>
|
@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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @jvdp1, thank you for resolving the conflicts with the updated codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @jvdp1 I also think this PR is ready to be merged.
Addition of
sort_adjto sort an rank 1 array in the same order as an input array.It is basically an extension of the original
sort_indexprocedure, by making the arrayindexintent(inout), instead ofintent(out)only.To be discussed:
sort_adj, but I think another (more appropriate) name should be foundindex: suggestions of a more appropriate name?sort_indexstill valid/appropriate/useful?@jalvesz
sort_adjcould replace your internal sorting procedure in your sparse implementation. If not, I think it is still a nice addition tostdlib.