Numpy frame

[giboul]

Hi everyone!

I spent too much time on the error below that showed up because I gave the frame argument of pyclaw.solution.Solution.__init__ a numpy integer instead of a pure python integer... So I thought it could be made a bit more flexible.

type(frame) = <class 'numpy.int64'>
Traceback (most recent call last):
  File "/home/giboul/Desktop/TriftGeoClaw/TSUL/read.py", line 114, in <module>
    read_cut(times[i])
  File "/home/giboul/Desktop/TriftGeoClaw/TSUL/read.py", line 71, in read_cut
    frame_sol = solution.Solution(frame, path=projdir/"AVAC"/"_output5", file_format=AVAC["out_format"])
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/giboul/Desktop/clawpack/pyclaw/src/pyclaw/solution.py", line 140, in __init__
    raise Exception('Invalid pyclaw.Solution object initialization')
Exception: Invalid pyclaw.Solution object initialization

With the suggested changes, the error message would look like this for an erroneous input:

Traceback (most recent call last):
  File "/home/giboul/Desktop/TriftGeoClaw/TSUL/read.py", line 114, in <module>
    read_cut(times[i])
  File "/home/giboul/Desktop/TriftGeoClaw/TSUL/read.py", line 71, in read_cut
    frame_sol = solution.Solution(frame, path=projdir/"AVAC"/"_output5", file_format=AVAC["out_format"])
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/giboul/Desktop/clawpack/pyclaw/src/pyclaw/solution.py", line 141, in __init__
    raise Exception(
Exception: Invalid pyclaw.Solution object initialization: '...' is not an int.

[coveralls]

coverage: 26.77% (-19.8%) from 46.556%
when pulling 1f0105a on giboul:NumpyFrame
into 37658ee on clawpack:master.

[mandli]

Yikes, that is annoying. I have run into the numpy integer problem a bunch lately as well.

My one thought now looking at this is that maybe we would rather simply cast the frame argument as an int(). This should raise a fairly straightforward exception and will allow other types to be converted if sane, *e.g. * int('3').

[giboul]

It would also accept floats, is that fine?

[ketch]

Thanks for taking the time to report and fix this, @giboul . I think it would be fine to merge this but even better to change the code to attempt to cast the input to int as @mandli says. It's hard to imagine a serious problem that that would cause.

[giboul]

Cool ! I just noticed I had not treated the case where frame is passed as a keyword.

[mandli]

Is there a reason not to just do frame = int(arg[0])?

[giboul]

Just that the error traceback is slighly more clear by explicitly saying frame is the issue:

    frame = int(frame)
            ^^^^^^^^^^
ValueError: invalid literal for int() with base 10: '_1_'

[mandli]

Hmm, good point. Not sure what is better I guess then.

[giboul]

Well... 'frame' still appears in frame = int(arg[0]) so I commited that change. Funny how it takes me 5 commits to change two lines.

[mandli]

The tests are failing as the Riemann commit does not include the fwave dictionary. Running it locally works though.

[giboul]

Is there something I can do about the tests failing?

[mandli]

We have to update the super-repository for clawpack with the updated commit point that includes the f-wave solver feature.

[mandli]

I think clawpack/clawpack#259 should take care of this although the interface has changed since last I did a PR like this. Once that get merged the testing should work.

[mandli]

This looks fine to me and works locally.