Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed orientation issue / rake is required to be set in set_corners() #323

Merged
merged 5 commits into from
Aug 12, 2018

Conversation

dsrim
Copy link
Contributor

@dsrim dsrim commented Aug 3, 2018

@rjleveque Thanks for the comments from github.com/rjleveque/geoclaw/pull/3. I've made some fixes in this PR.

[1-3] I think I mistakenly did a double-negative. I defined a logical self._fix_orientation: when it switches on (the orientation is clock-wise), I multiply a negative sign to fix the sign error. I agree that the data user input should be preserved, probably can confuse the user.

[4-5] I removed the optional rake=90. argument to self.calculate_geometry_triangles() and also made the rake input required for self.set_corners(), as suggested. (I think in the past I looked through all the triangular subfaults in CSZ and seeing them all having rake of 90 degrees I assumed that 90 degrees was the default for most cases.)

[6] I am not sure where to specify that the user needs to install utm and pyproj ... and yes, I think the discrepancy should be due to the various differences in transforming from long-lat to meters - there is actually a subtle issue here: for the triangular case, the method needs the deformations originating from each of the corners of the triangle to cancel exactly.

dsrim and others added 2 commits August 1, 2018 19:48
… set ``self._fix_orientation`` flag then fix the sign

- removed ``rake=90.`` as an optional input argument to ``self.calculate_geometry_triangles()`` and made ``rake`` required for ``self.set_corners()``
@rjleveque
Copy link
Member

Thanks @dsrim!

Looking again at the code, maybe it is best if set_corners does not take rake as an argument at all, since this function just sets self._corners and then calls calculate_geometry_triangles, which as far as I can see does not require the rake, only the corners. To apply okada the rake must be set, but so must the slip. Why set one in set_corners and not the other? It seems simplest to have set_corners just sets up the geometry doing what its name indicates, and then assume the user sets the rake and slip elsewhere.

Also, at some point we should try to fix this so that a vertical fault works too, but not urgent.

@dsrim
Copy link
Contributor Author

dsrim commented Aug 7, 2018

@rjleveque That makes more sense. I removed rake as input argument in self.set_corners and it is no longer set there. It's up to the user to set those.

As for the vertical fault - there's nothing mathematically that should prevent this... so I suspect there should be a nice fix. Let me get around to this at some point (hopefully there aren't any vertical subfaults in the meanwhile...)

@rjleveque
Copy link
Member

Thanks @dsrim! This looks good now except I notice I left in a couple debug statements that should be removed, the lines

            print('+++ initial strike_deg = %g' % strike_deg)
and
            print('+++ initial dip_deg = %g' % dip_deg)

@dsrim
Copy link
Contributor Author

dsrim commented Aug 9, 2018

@rjleveque debug statements removed!

@rjleveque rjleveque merged commit c2a37b2 into clawpack:master Aug 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants