-
Notifications
You must be signed in to change notification settings - Fork 20
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
base: master
Are you sure you want to change the base?
Conversation
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.
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.
|
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. |
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. |
handling could be improved there, it seems some calculations for valid output rely on nans getting produced via e.g. zero divisions.
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.
|
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: