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

Synchronize with obspy mopad #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbyt
Copy link

@mbyt mbyt commented Oct 15, 2013

For historical reasons, obspy ships it's own mopad version (https://github.com/obspy/obspy/blob/master/obspy/imaging/scripts/mopad.py). The two versions diverged quite a bit, which can be seen in this pull request, where I simply overwrote the geophysics/MoPaD version with the obspy MoPaD version: 3257 additions and 3161 deletions (unfortunately github does not show the diff, but they are easily visible locally).

There is another version of mopad in pyrocko https://github.com/emolch/pyrocko/blob/master/pyrocko/mopad.py.

Is there any plan for unification, such that obspy and pyrocko could include MoPaD as a submodule?

There were some performance improvements, bug fixes in the obspy version. Obspy.imaging also ships tests for MoPaD. Really merging the versions and not simply overwriting the one by the other could be a tough task.

References:

mbyt referenced this pull request in obspy/obspy Oct 15, 2013
Now adheres to PEP8. Also removed unused code parts and commented code.
Furthermore restructured the imports slightly and replaces some
expressions.

Tests still pass and I also tested it with lots of other moment tensors
which also look ok.
@emolch
Copy link
Collaborator

emolch commented Oct 17, 2013

The pyrocko.mopad module is completely outdated and I would be happy to replace or remove it from Pyrocko. Actually, I always used the pyrocko.moment_tensor module which is much simpler. Also I won't use mopad until it is split into several smaller and cleaner submodules.

I think the proposed overwrite with the obspy version is a good idea, as a starting point. I can start to work a bit on the split next week.

Lars ( @geophysics ), is the old svn repos of mopad still on some machine in Hamburg? I started to refactor and split that monster some years ago, but I cannot find it anymore... maybe my work is in the svn repos... Sorry, I have now seen the submodules branch...

@megies
Copy link

megies commented Oct 17, 2013

I didn't really look into it, but I wanted to point out that the mopad version here has a version number of 0.9 compared to the version number of 0.7 that is in the obspy codebase. So simply overwriting could mean losing some changes/improvements, I assume.

@emolch
Copy link
Collaborator

emolch commented Oct 21, 2013

Ok, I completed to merge the current obspy mopad version into the standalone 0.9b version of mopad (work done in branch true_merge_obspy_version in emolch/MoPaD). I locally tested the mopad test in obspy and it passes if one is willing to rename the 'system' argument to mopad MomentTensor() into 'in_system' as it is called in newer standalone mopad. However the obspy tests cover only very little of that horribly long mopad code so @geophysics should test this new version carefully before pulling it into MoPaD master.

megies added a commit to obspy/obspy that referenced this pull request Nov 3, 2013
handling could be improved there, it seems some calculations for valid
output rely on nans getting produced via e.g. zero divisions.
@emolch
Copy link
Collaborator

emolch commented Jan 20, 2014

Here's a script to compare beachballs produced with mopad, GMT and ObsPy at bitmap level. It fuzzes around some typically dangerous angles to trigger failures. imagemagick, ObsPy, gmtpy, pyrocko, mopad, and this beach_gmt.py are used.

difference mopad vs gmt: 41.0211%
difference mopad vs obspy: 1.57061%
strike, dip, rake: 90.000084573313202, 45.000000000000000, 180.000000000000000
mnn, mee, mdd, mne, mnd, med: -0.000002087496279, 0.000002087496279, 0.000000000000000, 0.707106781183466, -0.000001043748140, 0.707106781185777

fail-5

(mopad, gmt, obspy, |gmt - mopad|, |obpsy - mopad|) 

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.

None yet

3 participants