#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+--------------------------------------- Changes (by eschnett):

* priority: major => critical * milestone: => ET_2013_05

Comment:

It is not legal in Fortran to define the same module twice. The situation is the same as defining two routines with the same name -- this is a fundamental "don't". I am raising the severity of this. I also consider this a blocker for the release.

I understand that people strive to structure Fortran code in a manner similar to C code, but unfortunately that doesn't work. Instead of defining stand-alone subroutines, and then declaring interfaces (and repeating much code), it is far superior to define subroutines INSIDE a module, and then using that module when the subroutine is to be called. Include files should not be necessary at all for this.

The drawback of this is that Cactus cannot call subroutines that are defined in a module, it only knows how to call stand-alone subroutines (however, this can be changed). That is, all scheduled or externally visible routines need to be stand-alone routines.

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+---------------------------------------

Comment (by rhaas):

nothing tested, one solution is to remove the module altogether. Another to remove the module part of the file and have the #include inside of each subroutine that uses the interfaces.

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: new Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+---------------------------------------

Comment (by rhaas):

In particular we have duplicate modulde definition in Con2PrimInterfacesM.h and Con2PrimInterfacesAM.h which prevents (depending on some race condition) compilation of the code.

I do not think this should be left alone instead we should fix it. Even if the "fix" is removal of the module altogether. It's not useful anyway since it tries to define a module for subroutines that are implemented in C, I don't even understand how it ever compiled.

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+--------------------------------------- Changes (by rhaas):

* status: new => review

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+---------------------------------------

Comment (by rhaas):

yes the release branch one replicates the changes in rev 517 on trunk which do not apply to 2013_05 since 517 depends on 511 which is not yet in 2013_05. The later is the one that should restore being able to run the magnetized collapse with trunk/release branches. I am as of yet not sure though if that is really the case yet. The changes to src/GRHydro_Con2PrimAM.F90 commute with the change in module location, *however* one needs to remove use of the include file, so interface-2013_05.patch is really two serial changes: 1.) do the equivalent of r517 in trunk, 2.) remove .h file in favour of proper Fortran files.

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: review Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: | Keywords: GRHydro ------------------------------------+---------------------------------------

Comment (by rhaas):

Tested on my workstation with gcc and on bethe with intel13. So two machines and the two compilers but never on the same machine with both compilers.

I am somewhat unhappy to see trunk and release diverge already because of 517, but don't quite see a way to avoid it (well I could slim down the interfaces-2013_05.patch file and really only remove the #include I guess thus having fewer changed lines).

#1333: Fortran module in GRHydro_InterfacesM.h is nonsensical ------------------------------------+--------------------------------------- Reporter: rhaas | Owner: Type: defect | Status: closed Priority: critical | Milestone: ET_2013_05 Component: EinsteinToolkit thorn | Version: Resolution: fixed | Keywords: GRHydro ------------------------------------+--------------------------------------- Changes (by rhaas):

* status: review => closed * resolution: => fixed

Comment:

Oh right, it does say "please apply". Applied the trunk patch as rev 520, applied a "interface only" patch as rev 521 of 2013_05. I'll worry about Con2PrimAM.F90 getting out of sync with Con2PrimM.F90 at another time (occasionally I get the feeling that GRHydro's code could be vastly improved by a rm -rf * :-) ).