Skip to content

Commit

Permalink
Add regression test for 172 bad co_reduce
Browse files Browse the repository at this point in the history
 [issue #172](#172)
 causes co_reduce to return wrong results if the binary operator has
 arguments declared with the `value` attribute rather than
 `intent(in)`. Switching to use `intent(in)` is a valid work
 around. The binary operator is required to be a pure function with
 two arguments. As @LadaF points out, F2008 says:

 > C1276 The specification-part of a pure function subprogram shall
   specify that all its nonpointer dummy data objects have the
   `INTENT(IN)` or the `VALUE` attribute.

 Original coverage diff from adding this test was at:
 https://codecov.io/gh/sourceryinstitute/opencoarrays/compare/882c371d4d7e84364eb7adfba4b4f8d840e3f398...1a0e3b7edb6867d9e9371cbd9ed1d8f5b3dd2010/changes

 If this commit produces a different change in coverage then that
 likely indicates where there is a bug in the library. If the library
 coverage remains the same, then the bug is probably in the
 compiler. See discussion at
 [#172](#172)
  • Loading branch information
zbeekman committed Jan 20, 2017
1 parent 7e1ceb4 commit 625b8b7
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 3 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Expand Up @@ -435,6 +435,7 @@ if(opencoarrays_aware_compiler)
endif()
add_mpi_test(convert-before-put 3 ${tests_root}/regression/reported/convert-before-put)
add_mpi_test(event-post 3 ${tests_root}/regression/reported/event-post)
add_mpi_test(co_reduce-factorial 4 ${tests_root}/regression/reported/co_reduce-factorial)
else()
add_test(co_sum_extension ${tests_root}/unit/extensions/test-co_sum-extension.sh)
set_property(TEST co_sum_extension PROPERTY PASS_REGULAR_EXPRESSION "Test passed.")
Expand Down
1 change: 1 addition & 0 deletions src/tests/regression/open/issue-172
9 changes: 6 additions & 3 deletions src/tests/regression/reported/CMakeLists.txt
@@ -1,9 +1,12 @@
add_executable(co_reduce-factorial issue-172-wrong-co_reduce.f90)
target_link_libraries(co_reduce-factorial OpenCoarrays)

add_executable(source-alloc-sync issue-243-source-allocation-no-sync.f90)
target_link_libraries(source-alloc-sync OpenCoarrays)

add_executable(convert-before-put issue-292-convert-type-before-put.f90)
target_link_libraries(convert-before-put OpenCoarrays)

add_executable(event-post issue-293-silent-event-failure.F90)
target_link_libraries(event-post OpenCoarrays)

add_executable(source-alloc-sync issue-243-source-allocation-no-sync.f90)
target_link_libraries(source-alloc-sync OpenCoarrays)

63 changes: 63 additions & 0 deletions src/tests/regression/reported/issue-172-wrong-co_reduce.f90
@@ -0,0 +1,63 @@
program co_reduce_factorial
!! author: Daniel Topa & Izaak Beekman
!! category: regression
!!
!! [issue #172](https://github.com/sourceryinstitute/opencoarrays/issues/172)
!! wherein co-reduce gets junk in the first image when binary
!! operator's (pure function) arguments have `value` attribute
!! instead of `intent(in)`

implicit none
integer :: value[ * ] !! Each image stores their image number here
integer :: k
value = this_image ( )
call co_reduce ( value, result_image = 1, operator = myProd )
!! value[k /= 1] undefined, value[ k == 1 ] should equal $n!$ where $n$ is `num_images()`
if ( this_image ( ) == 1 ) then
write ( * , '( "Number of images = ", g0 )' ) num_images ( )
do k = 1, num_images ( )
write ( * , '( 2( a, i0 ) )' ) 'value [ ', k, ' ] is ', value [ k ]
write ( * , '(a)' ) 'since RESULT_IMAGE is present, value on other images is undefined by the standard'
end do
write ( * , '( "Product value = ", g0 )' ) value !! should print num_images() factorial
write ( * , 100 )
if ( value == factorial( num_images() ) ) then
write ( * , '(a)' ) 'Test passed.'
else
write ( * , '(a, I0)') 'Answer should have been num_images()! = ', factorial( num_images() )
error stop 'Wrong answer for n! using co_reduce'
end if
end if
100 format ( "Expected value = num_images()!", /, " 2! = 2, 3! = 6, 4! = 24, ..." )

contains

pure function myProd ( a, b ) result ( rslt )
!! Product function to be used in `co_reduce` reduction for
!! computing factorials. When `value` attribute is changed to
!! `intent(in)` tests pass, and expected behavior is observed.
integer, value :: a, b
!! multiply two inputs together. If we change `value` to
!! `intent(in)` the test passes and the issue goes away and
!! according to C1276 of F2008:
!!
!! > C1276 The specification-part of a pure function subprogram
!! > shall specify that all its nonpointer dummy data objects have
!! > the INTENT (IN) or the VALUE attribute.
!!
!! Thanks to @LadaF for pointing this out.
integer :: rslt !! product of a*b
rslt = a * b
end function

pure function factorial ( n ) result ( rslt )
!! Compute $n!$
integer, intent(in) :: n
integer :: rslt
integer :: i
rslt = 1
do i = 1, n
rslt = rslt*i
end do
end function
end program

0 comments on commit 625b8b7

Please sign in to comment.