fix to geoclaw riemann solver for convergence rate test

[dlgeorge]

fixed bug: fw(3,3)--> fw(3,2) in augJCP solver, confirmed for kyles vorticity problem

[mandli]

Are we ok with merging this? I have tested on a number of problems and it seems to work great, including the ones testing for convergence.

[dlgeorge]

sounds good to me. I've been working on some other clean-ups of the Riemann solver (not on this branch)..but maybe those can wait for later and more testing.

[mandli]

Oh, is it something that would be ready soon? I am not sure that delaying a bit would matter.

[dlgeorge]

hopefully pretty soon (I'm sort of experimenting in the dclaw one and have made some improvements I think), but it's sort of experimental, so should probably be tested thoroughly. One option could be to keep the current one (this PR) and then update if the experimental works. But, I might have a good idea how robust it is this week (working on it now). ...so maybe short delay and then decide (?).

[mandli]

Well, if you would like to merge this one and do a new PR that would make sense to me as well. May make it easier to test fully. Up to you.

[mandli]

@dlgeorge What's the status on this? Should we merge this in now and test the other improvements in another PR?

[mandli]

I think we should merge this and open a new PR for the additional enhancements.

[rjleveque]

@mandli, Sorry for not looking at this sooner. I just tried the new version on the chile2010 example and it gives visually identical results, although diffing the outputs shows large relative differences in some values (e.g. in the first or second digit) but only where the values are quite small, so absolute differences are negligible (e.g. < 1 mm in eta).

I was just wondering if you've tested this much with inundation problems? It's presumably fine, but I'm alway nervous when we make a change in the core routines used for doing inundation modeling in past hazard studies, so I will try a few tests too.

But it's great that you and @dlgeorge found a bug that improves the converence rate, so, this is presumably an improvement over the previous code. Thanks!

[mandli]

I share the same concerns. The tests I ran showed nearly identical results as you found. We were hoping that the changes were either computational or clearly a better answer, but we need to get that testing suite back up to do some of these tests more systematically.

[rjleveque]

Yes, we need to get pytest working. But also I think we should test on some big production problems to see if results changed in any significant way. I will try some tests when I get a chance...

[mandli]

I am working on one related to storm surge now as well.