Menu

#60 Failed assertion in subroutine called from test should prevent rest of test from executing

Development v3.x
open
nobody
None
5
2016-09-23
2016-09-23
Bill Sacks
No

See attached file for an example of this problem: When a test routine calls some other (helper) subroutine, and that helper subroutine contains an assertion, I would expect a failure in that assertion to cause the test to bail and stop executing - just as an inline assertion in a test would. Instead, the failed assertion in the test helper causes the test helper to bail, but the main test routine continues.

If there were no other problems in the test, then an error message is printed from the failed assertion, as expected. However, this is an issue if there are later problems in the test: I would have expected the later problems to be skipped over due to the failed assertion, but that is not the case.

So, in the attached example, I see a printed message saying 'start of helper' (but NOT 'end of helper' - that's correct), but then the test aborts with:

At line 19 of file /Users/sacks/cime/driver_cpl/unit_test/pfunit_test/mytest.pf
Fortran runtime error: Attempt to DEALLOCATE unallocated 'myvar'

whereas I would have expected it to abort with the assertion error message (which is what happens if the assertion is inline in subroutine test1).

In practice, I have run into this problem when I use a test helper routine to do some of the setup for my tests (allocating variables, etc.): I put assertions in the test helper to ensure that it's being called correctly from the individual tests (e.g., array sizes that agree, etc.). If one of these assertions fails, I want the whole test to fail immediately. Instead, the test helper returns at that point and the rest of the test routine continues - causing unexpected results because the setup routine didn't get to do its thing. So I see a seg fault or some other error later in the test, rather than seeing the assertion message I had wanted to see.

For now I can work around this by not using @assert calls in test helper methods. And I can continue to do so if it turns out that the current behavior is a feature rather than a bug.

This is with an mpi build of pfunit on a Mac, using gfortran. I can't find the exact version number, but it looks like I built it from the latest version available as of Feb 12, 2016.

1 Attachments

Discussion

  • Tom Clune

    Tom Clune - 2016-09-23

    Unfortunately, this is a limitation of Fortran, which does not have true exceptions. All that the @assert() annotation can do is introduce a RETURN statement in the current procedure. Presuming that you really want the helper procedure, you have two choices:
    1. As you noted above, have the helper procedure return a status variable which is then used in an @assert() in the actual test procedure.
    2. Add an additional check in the test procedure to see if the helper procedure threw an exception:

    ...
    call helper() ! might throw exception
    if (any_exceptions()) return
    deallocate(myvar)
    

    Note that if the test is an MPI test, then you may need to guard against the possibility that some processes experienced exceptions and others did not. Usually you will be fine with the @mpiAssert() annotation, but if not, thene you can pass the test object's 'context' component to any_exceptions():

    if (any_exceptions(this%context)) return

    That statement will issue RETURN on all processes if any process experienced an exception. Note that it is blocking - so it must be someplace that all processes are guaranteed to reach.


    I know that this is all a bit inelegant, and I'd like a better solution myself. But I am unaware of any reasonably simple approach to get around such issues. I am quite open to suggestions, that I would happily incorporate into 4.0.

     
  • Bill Sacks

    Bill Sacks - 2016-09-23

    Hi Tom,

    Thanks a lot for your detailed reply. Yes, this makes sense to me - thanks for explaining this.

    I agree that I can't think of an easy way around this problem. I guess you could handle this by having some annotation in the caller, like:

    call helper()
    @check
    

    which would just get turned into your suggested:

    call helper()
    if (any_exceptions()) return
    

    but that doesn't really buy you much - and I don't really like requiring the caller to know if the routine it's calling might possibly have an assertion, either directly or indirectly (via something that it calls).

    I thought about having the preprocessor insert

    if (any_exceptions()) return
    

    after each call statement. But that would only handle subroutines, not functions - and a partial solution like that might be worse than no solution at all.

    Then I thought about taking the drastic measure of having the preprocessor insert

    if (any_exceptions()) return
    

    after every line of executable code in a .pf file, but I imagine that could be a preprocessing nightmare to cover all the edge cases.

    Since this doesn't seem like an easily-solvable problem, I'm fine with your closing this bug report. I'll just remember not to use assertions in this way.

    Thanks again.

     
  • Tom Clune

    Tom Clune - 2016-09-23

    Yes - I like the concept of @check. There would need to be an @mpiCheck as well. Much better than the one I almost suggested in my first message: @assertNoExceptions

    Unfortunately, even if we restrict to subroutines, there would be issues with inserting @check after every statement. The problem is that it can be subtle in an MPI context whether or not to have a collective operation. E.g., if there is a branch like:

    if (this%getProcessRank() ==0) then
        call helper1()
        call helper3()
    else
        call helper2()
    endif
    

    Here it would be essential to use @check rather than @mpiCheck, as not all processes would reach all statements. But in other cases, @check can lead to problems where one ore more processes have abandoned the test but others are proceeding. In my experience these issues are rare - I tend to write the tests in a way that it does not arise, but they cannot always be avoided.

    And as you say doing this in the preprocessor would be difficult without a more sophisticated approach than the one I'm currently using.

     

Log in to post a comment.