Fortran-FOSS-Programmers / FOODIE

Fortran Object-Oriented Differential-equations Integration Environment, FOODIE
127 stars 30 forks source link

Pure procedures with intent(inout) dummy arguments #10

Closed milancurcic closed 9 years ago

milancurcic commented 9 years ago

Compiling FOODiE with Cray compiler on an XC-30 reveals the following (possible) bug:

ftn -c  type_integrand.f90

module type_integrand
       ^              
ftn-855 crayftn: ERROR TYPE_INTEGRAND, File = type_integrand.f90, Line = 2, Column = 8 
  The compiler has detected errors in module "TYPE_INTEGRAND".  No module information file will be created for this module.

  pure subroutine assignment_integrand(lhs, rhs)
                                       ^         
ftn-1265 crayftn: ERROR TIME_DERIVATIVE, File = type_integrand.f90, Line = 82, Column = 40 
  Non-pointer dummy argument "LHS" to PURE FUNCTION "TIME_DERIVATIVE" must have INTENT(IN) specified for it.

  pure subroutine assignment_real(lhs, rhs)
                                  ^         
ftn-1265 crayftn: ERROR TIME_DERIVATIVE, File = type_integrand.f90, Line = 92, Column = 35 
  Non-pointer dummy argument "LHS" to PURE FUNCTION "TIME_DERIVATIVE" must have INTENT(IN) specified for it.

Cray Fortran : Version 8.3.11 (u83061f83225i83184p83333a83009e83011z83333)
Cray Fortran :                (x8321r83015w83011t8311b83039)
Cray Fortran : Thu Aug 13, 2015  22:37:03
Cray Fortran : Compile time:  0.0080 seconds
Cray Fortran : 103 source lines
Cray Fortran : 3 errors, 0 warnings, 0 other messages, 0 ansi
Cray Fortran : "explain ftn-message number" gives more information about each message.
make: *** [type_integrand.o] Error 1

Apparently, Cray does not like this. Intel and GNU are fine with it. I do not know from the top of my head whether this is allowed by the standard, and it is too late in the evening for me to go look read through the fineprint.

The code compiles without error if the pure attribute is removed for the following procedures: function time_derivative, subroutine assignment_integrand, subroutine assignment_real.

szaghi commented 9 years ago

Wow... I have to check, but if I remember right the standard states that pure procedure are procedures without side effect, a pure subroutine should be allowed to assing a dummy argument thus it should be allowed to hsve lhs declared as intent(inout). Anyhowm tgank you very much for the test, it is very aporeciated!

zbeekman commented 9 years ago

If it is indeed a function, all dummy arguments must be intent(in) for it to be pure. If it's a subroutine it may have intent(out) or intent(inout) dummy arguments as well. The Cray compiler is correct.

szaghi commented 9 years ago

If it is indeed a function, all dummy arguments must be intent(in) for it to be pure. If it's a subroutine it may have intent(out) or intent(inout) dummy arguments as well. The Cray compiler is correct.

There is somenthing that I do not see... the function time_derivative has only self that is intent(in) (as all other functions that have only intent(in) arguments), the subroutines for the assigments must be allowed to have intent(inout)... in my opinion the compiler is wrong. However I cannot now check the standard, I am out of office.

zbeekman commented 9 years ago

Yes, it appears that the compiler is confused... the code is indeed legal AFAICT. This is actually quite alarming, because the error messages complain about the TIME_DERIVATIVE function but then give line numbers and print out different (assignment) subroutine declarations.

Maybe you need to put each abstract interface within it's own abstract interface block as a compiler bug work around? Or give the dummy arguments unique names? Or move the function last?

This is 100% a compiler error, though.

milancurcic commented 9 years ago

@zbeekman @szaghi We need to read the error message very carefully here.

ftn-1265 crayftn: ERROR TIME_DERIVATIVE, File = type_integrand.f90, Line = 82, Column = 40 
  Non-pointer dummy argument "LHS" to PURE FUNCTION "TIME_DERIVATIVE" must have INTENT(IN) specified for it.

The problem is not that the subroutines have intent(inout) for their dummy argument. And as @szaghi points out, time_derivative has only intent(in), so that is not the problem either.

The problem is that he lhs dummy argument in subroutines assignment_integral and assignment_real, wants to take the pure function time_derivative as its intent(inout) dummy argument. In fact, only removing the pure attribute from function time_derivative makes the code compile without errors.

I don't see why this would be illegal though, and as I type this, @zbeekman just posted some good suggestions for workarounds. :)

szaghi commented 9 years ago

Maybe you need to put each abstract interface within it's own abstract interface block as a compiler bug work around? Or give the dummy arguments unique names? Or move the function last?

Great Zaak! nice suggestions, unfortunately I have not acces to cray compiler for check this workarounds, but split the interface sounds a more sane approach for my OCD! As always, thank you!

zbeekman commented 9 years ago

@milancurcic I'm not sure I understand what you're saying here:

The problem is that he lhs dummy argument in subroutines assignment_integral and assignment_real, wants to take the pure function time_derivative as its intent(inout) dummy argument.

They are taking class(integrand) as their intent(inout) argument. Despite the uses in the concrete implementation down the road, the compiler is only away aware of the abstract interfaces and classes when it is compiling type_integrand.f90.

At any right, I think we're all on the same page that this is a bug that needs to be reported to Cray, as it is very fundamental. @milancurcic how recent is the Cray compiler you used to test this?

milancurcic commented 9 years ago

@zbeekman I am aware how the dummy arguments are declared in the subroutines - I was pointing out what seems like the compiler is trying to do. I as well think the compiler is confused.

I don't know how old is this particular version, as I'm not familiar with Cray's timeline. I suspect it is not too old (< 12 months), as it is used for production on a large DOE system, and in general they've been keeping things up-to-date.

I will be able to do a few more tests over the weekend and submit a ticket. I am not 100% convinced yet that this has to be a compiler bug, but I'm also quite fresh with a lot of OO implementations in Fortran. I hope Damian (@rouson) can chime in on this.

milancurcic commented 9 years ago

Reproduced the same error with Cray Fortran 8.4.0.219. Note that this version was in beta until last week, so should be fairly recent.

zbeekman commented 9 years ago

Thanks for looking into this. One way to help confirm that it is a compiler bug would be to find a work around, that should be semantically equivalent. I think that the compiler is just having trouble with multiple declarations inside a single abstract interface block, which is completely legal.

milancurcic commented 9 years ago

OK, I will be able to test your interface suggestion over the weekend.

milancurcic commented 9 years ago

I tried recompiling after placing individual abstract interface blocks, as suggested by @zbeekman. Also tried placing the time_derivative function interface at the end. Both fail to compile with the same error message as above.

I submitted the ticket to the NERSC help desk. They will follow-up with a bug report with Cray.

szaghi commented 9 years ago

@milancurcic Thank you very much!

szaghi commented 9 years ago

Hi all,

I do not know the status of this bug, anyhow in the present version v0.0.8 all the type bound procedures of the type_integrand are no more defined pure.

I eliminated the pure attribute for supporting parallel architectures: for example, it is not allowed to have a pure procedure using parallel OpenMP directives, they being impure. Maybe the same can happen with the MPI library.

I think this issue could be closed, let me know.

See you soon.

milancurcic commented 9 years ago

@szaghi OK, let me test the updated code with Cray compilers and get back to you, probably next week. Thank you for all the rapid updates to the code.

szaghi commented 9 years ago

@milancurcic Thank you too!

milancurcic commented 9 years ago

While there has been no updates from Cray on this bug, I can confirm that the updated code (without the pure attributes for the type_integrand methods) compiles with the Cray compiler. Closing.

szaghi commented 9 years ago

Thank you Milan!