adjtomo/pyadjoint

adopt or merge DD and envelope misfit functions from computational-seismology

Closed this issue · 4 comments

bch0w commented

There is a separate fork of PyAdjoint which contains a significant amount of development which is not present here. From what I can see these include double difference (DD) and exponentiated phase misfit functions. I think there are also a number of bug fixes in that branch that are not present here. Although I am not aware of specifics.

https://github.com/computational-seismology/pyadjoint/tree/dev

I was thinking we should try to merge these changes into this main repository so that they're accessible to all, and usable for a SeisFlows or Pyatoa workflow.

Looking through the commit history, @chukren @yanhuay @wjlei1990 @rdno are the main contributors. Questions for you all:

  • Are you using the computational-seismology version of Pyadjoint?
  • Do you have any objection with attempting to merge these two repositories, or somehow co-opting those changes in this repository?
  • Is there any summary of diverging changes made, besides the commit history?
rdno commented

Hi Bryant,

This is probably not the exhaustive list but aside from the things you mentioned,

  • There was some effort to benchmark multitaper measurements with MEASURE_ADJ fortran code. Some changes made accordingly.
  • Each misfit has its own Config class (ConfigMultiTaper, ConfigWaveform etc.)
  • I think it has also more tests.

I'm currently using my own fork rdno/pyadjoint:py3. I've noticed that when we merged the exponentiated phase measurement, we forgot to reverse the adjoint source. That's fixed in my fork and I've added waveform_DD measurement. I've also added support for computing anelastic adjoint sources although that feature needs some cleanup.

I can't speak for the others but I have no objection for the merge. Maintained, up-to-date and full-featured central repository would be a good thing because I don't think anyone is maintaining the computational-seismology version. I'm not sure how you are planning to do it because it might introduce some breaking changes for Seisflows and Pyatoa. I'm willing to help if you need it.

bch0w commented

Thanks for the response Ridvan! Awesome to hear about the tests and benchmarking.

All of this sounds like very useful additions to have in the central repository. Thanks for pointing me to your current fork. Maybe what I'll do is attempt to merge your version with mine locally to see how much work will be required to fix merge conflicts and get things working with Pyatoa.

If the conflicts are small I am happy to take care of them, but if they are large then maybe I will start a pull request with some major tasks outlined and we can work at it together.

bch0w commented

Hi Ridvan (@rdno). Just notifying you that I've started on this package merge in the package_refactor branch

https://github.com/adjtomo/pyadjoint/tree/package_refactor

I'm keeping the CHANGELOG in that branch up to date. I've updated the Config to match your fork. I also cleaned up the existing misfit functions, in particular I overhauled the multitaper misfit as it was pretty messy. The waveform and CC misfit functions have been benchmarked with the test data against your fork of pyadjoint.

Next up will be to benchmark the MTM changes, and then start incorporating the new misfit functions.

bch0w commented

Closed with #8