Rename ierr and serr

[aprokop]

Leaving them unprefixed will lead to collisions. This also does not follow xSDK policies. They should be renamed to something like fortrilinos_ierr and fortrilinos_serr instead.

@sethrj @tjfulle @kevans32 Thoughts?

[tjfulle]

I totally agree. Once a public release is made, making this change could be extremely disruptive. I think the change should be made in conjunction with #119. It should be unambiguous where fortrilinos_ierr originates.

[tjfulle]

On this same topic. I believe that fortrilinos_ierr and fortrilinos_serr should be defined in their own module (#119), as previously stated, but also, we should provide site-wide ForTrilinos.h header to include in any source. This header would use the module where fortrilinos_ierr lives. This header should also provide a CHECK_IERR macro that will check the value of fortrilinos_ierr and stop if it is not 0. User code would have something like

#include "ForTrilinos.h"

blah, blah, blah

call SomeForTrilinosProcedure(); CHECK_IERR()

Similar functionality exists in the FortranTestMacros.h and FortranTestUtilities.h files, but I am proposing that CHECK_IERR be separate from the testing frameworks and have a more direct relationship with fortrilinos_ierr. Several versions of CHECK_IERR could be provided, depending on if the compiler supports more than 132 characters or not (#153).

Incidentally, this is pretty similar to the typical usage of fortran wrappers to PETSc

[aprokop]

@tjfulle Sounds good! I'll try to do that while you are working on testing.

[tjfulle]

What would you think about making a fortrilinos module that managed ierr and serr, but that those variables are private. Getter/setter methods would be used to access them. ForTrilinos.h would provide convenience macros to the getter method that would stop calculations if an error is registered. That would reduce the global state floating around

[aprokop]

@tjfulle I started working on some ierr/serr stuff. I have put WIP in PR #169. So far, it implements forerror module that has public serr and ierr. We can tweak it further. The problem right now is that something is wrong and it segfaults.

[aprokop]

I don't know how I feel about getter/setter methods. I guess I would need to see an example of how it would be used and weigh against current use.

[tjfulle]

I was just thinking of how to reduce global data. subroutine set_ierr(ierr, seer) would set private module data, etc. it’s not that big a deal I guess.

[aprokop]

@tjfulle Can you give an example of how the code would look like?

Another thing right now is that the use of ierr in macros is ambiguous. I think there should be two different ierr, one coming from fortrilinos modules, and one from the test runtime.

[tjfulle]

Excuse the formatting, written on my phone from a ski lift

Module fortrilinos

Integer, private :: fortrilinos_ierr = 0
Character(Len=255), private :: fortrilinos_serr = ‘’
Subroutine set_error_state(errno, errstr)
  Integer, intent(in) :: errno
  Character(len=256), intent(in) :: errstr
  fortrilinos_ierr = errno
  fortrilinos_serr = errstr
End subroutine

Subroutine get_error_state(errno, errstr)
  Integer, intent(out) :: errno
  Character(len=256), intent(out) :: errstr
  errno = fortrilinos_ierr
  errstr = fortrilinos_serr
End subroutine

subroutine fortrilinos_check_ierr(filename, line, ...)
  ...
End subroutine

#define CHECK_IERR() call fortrilinos_check_ierr(__FILE__, __LINE__)

[aprokop]

Excuse the formatting, written on my phone from a ski lift

@tjfulle This is the best thing I saw on github ever!

[tjfulle]

Back again!

To expand a bit, I prefer limiting access to module wide variable through public module procedures. The macro CHECK_IERR can interface with fortrilinos_check_error_state. Different versions can be defined with/without __FILE__, depending of the compiler supports long lines. The macro TEST_IERR would also interact with fortrilinos_check_error_state, but send in the file name given in the test file.

[aprokop]

Feel free to update #169 with your ideas. Once you have a working prototype, I'll see if I can massage SWIG to work together with it.

[sethrj]

@aprokop define SWIG_FORTRAN_ERROR_INT to the integer error value (e.g. fortrilinos_ierr); and SWIG_FORTRAN_ERROR_STR for the string accessor function

[aprokop]

For the most part, fixed in #169.