clawpack / clawutil

General utility programs
BSD 3-Clause "New" or "Revised" License
10 stars 31 forks source link

Fix python2 incompatibility #138

Closed mandli closed 5 years ago

mandli commented 5 years ago

This addresses Python 2.x incompatibilities in PR #136 and includes #137 fixes.

mandli commented 5 years ago

@rjleveque I think this fixes everything but can you check when you get a chance and then retry the GeoClaw PR?

rjleveque commented 5 years ago

It seems to be running now, stay tuned...

However, when I first tried to run it I was missing an fgmax input file in this directory and so it threw a fortran error, and several Python errors in the process of doing so. Maybe this is the expected behavior in this case, but it may make it harder for the user to figure out what the real error was. Is it possible to print out a more useful message saying the fortran code died rather than looking like a Python error in this case?

(Note to self: change old to unknown in opening the fgmax file from fgmax_read.f90 so this prints the message it is supposed to rather than throwing a fortran error.)

Reading data file: fgmax.data
         first 5 lines are comments and will be skipped
At line 47 of file /Users/rjl/clawpack_src/clawpack_master/geoclaw/src/2d/shallow/fgmax_read.f90 (unit = 45)
Fortran runtime error: Cannot open file '/Users/rjl/git/WA_EMD_2018/RANDY/bainbridge/eagle_harbor_test_faster_filpatch/fgmax_eagle_harbor.txt': 
  No such file or directory

Error termination. Backtrace:
#0  0x1086c46fd
#1  0x1086c5395
#2  0x1086c5b29
#3  0x108889968
#4  0x108889c5c
#5  0x108680b8f
#6  0x10863c79c
#7  0x10867fbcf
#8  0x10868f21e
Traceback (most recent call last):
  File "/Users/rjl/clawpack_src/clawpack_master/clawutil/src/python/clawutil/runclaw.py", line 227, in runclaw
    stderr=xclawerr)
  File "/Users/rjl/miniconda/envs/geo/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/Users/rjl/git/WA_EMD_2018/RANDY/bainbridge/eagle_harbor_test_faster_filpatch/xgeoclaw']' returned non-zero exit status 2.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/rjl/clawpack_src/clawpack_master/clawutil/src/python/clawutil/runclaw.py", line 322, in <module>
    runclaw(*args)
  File "/Users/rjl/clawpack_src/clawpack_master/clawutil/src/python/clawutil/runclaw.py", line 232, in runclaw
    stderr=cpe.stderr)
__main__.ClawExeError: error
make: *** [output] Error 1
mandli commented 5 years ago

That's a good idea Randy. We can check to see if the Fortran returned a non-zero code I think and make that more clear.

mandli commented 5 years ago

I added

        print("")
        print("*** FORTRAN EXE FAILED ***")
        print("")

before the re-raise of the ClawExeError. We should probably keep this as one could want to handle the exception somehow.

bolliger32 commented 5 years ago

I added

        print("")
        print("*** FORTRAN EXE FAILED ***")
        print("")

before the re-raise of the ClawExeError. We should probably keep this as one could want to handle the exception somehow.

I think this makes sense, but is there any reason not to replace the text of the error (currently just 'error') with this instead of printing it beforehand? Either way, one can access the stderr stream of the executable from the ClawExeError if you catch it.

mandli commented 5 years ago

Right now it's printed above the exception. I think the real issue is that the traceback is the last thing printed rather than the error but I am not sure how to get around this.

ketch commented 5 years ago

It's worth remembering that Numpy and Matplotlib are committed to dropping Python 2 support by Jan. 1, 2020. This will essentially force us to do the same.

bolliger32 commented 5 years ago

Right now it's printed above the exception. I think the real issue is that the traceback is the last thing printed rather than the error but I am not sure how to get around this.

It looks like the 2nd to last thing is __main__.ClawExeError: error. If we just changed the error message from 'error' to something like what you printed above, they'd read it when they looked at the error I think.

mandli commented 5 years ago

That's a good idea. Just modified the code.

rjleveque commented 5 years ago

Works for me and ok to merge as far as I'm concerned. I like having it print something at the end pointing to the fortran error.

I also just fixed the problem in fgmax_read in https://github.com/clawpack/geoclaw/pull/399, so it now prints a sensible message and quits rather than throwing a Python error at all.