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

Local file tutorial #468

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

tjnewton
Copy link
Contributor

@tjnewton tjnewton commented Aug 6, 2021

What does this PR do?

Contributes a local file tutorial built from the quick_start.ipynb tutorial.

Why was it initiated? Any relevant Issues?

Issue #428

PR Checklist

- [ ] develop base branch selected?

  • This PR is not directly related to an existing issue (which has no PR yet).
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
  • Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGES.md.
  • First time contributors have added your name to CONTRIBUTORS.md.

@calum-chamberlain calum-chamberlain changed the base branch from develop to master August 6, 2021 01:22
@calum-chamberlain
Copy link
Member

Thanks for this @tjnewton! I will take a look at this in a moment. I just wanted to say thanks and point out that I changed the base to master because your branch was based on master, and I think that this should get through into the docs asap (which it will do as soon as we merge to master, but wouldn't until the next release if we merge to develop because of how I manage the branches) - sorry for the confusion!

@calum-chamberlain
Copy link
Member

Would you be able to add some text around the code for this? I'm happy to break do this if you want. Also - can this be run with a slightly smaller dataset? Downloading 30 days of data is quite a lot for a tutorial.

It is worth knowing that all the tutorials get run by our CI services to check the output matches what we expect, so there will need to be some checking that the outputs are consistent, and that the tutorial can be run in a timely manor. Our docs builder also re-runs the tutorials to generate the rendered webpage, so again, having a relatively compact example will make building docs, and running tests much more tractable.

Also, is the temaplate a real event? The waveforms of the template and the detection plotted are not particularly nice. It might also be good to have a few more template events - It might be cleaner to provide a quakeML (or similar) with the catalogue rather than making hypocentre formatted file in the notebook?

@tjnewton
Copy link
Contributor Author

tjnewton commented Aug 6, 2021

Sure thing, I will add some text. Those templates were LFEs so they are obscure. I just pushed an update using a larger amplitude event and only one day of data. I think it's useful to include building a catalog manually because it wasn't originally clear to me how to do this and make it work with EQcorrscan, so I think this will help others that are working with custom event catalogs. I can edit to pull an event catalog via Obspy if you disagree.

@calum-chamberlain
Copy link
Member

Great, thanks @tjnewton! I will have a proper read through of that this week - I'm just trying to get docs building automatically for this PR so that we can check that this renders okay on the readthedocs site, so I'm going to push a minimal change commit to (hopefully) trigger a build.

As far as creating an ObsPy catalogue goes - in general reading from some known phase format makes sense, but if you don't have a file with a known phase format then making the Event objects in memory can be safer (in that you do not need to get the formatting perfect, and it is just normal Python code). When I have to make a Catalog from other information I would do something like (but in a loop, and without hard-coding everything!):

from obspy.core.event import (
    Catalog, Event, Pick, Origin, Magnitude, WaveformStreamID)
from obspy import UTCDateTime
    
event = Event(
    origins=[Origin(
        latitude=61.9833, longitude=-144.0437, depth=1700, 
        time=UTCDateTime(2016, 9, 26, 8, 52, 40))],
    magnitudes=[Magnitude(mag=1.1)],
    picks=[
        Pick(time=UTCDateTime(2016, 9, 26, 8, 52, 45, 180000), phase_hint="P",
                waveform_id=WaveformStreamID(
                    network_code="YG", station_code="RH08", channel_code="BHZ")),
        Pick(time=UTCDateTime(2016, 9, 26, 8, 52, 45, 809000), phase_hint="P",
                waveform_id=WaveformStreamID(
                    network_code="YG", station_code="NEB1", channel_code="BHZ")),
        Pick(time=UTCDateTime(2016, 9, 26, 8, 52, 45, 661000), phase_hint="P",
                waveform_id=WaveformStreamID(
                    network_code="YG", station_code="NEB3", channel_code="BHZ"))])

catalog = Catalog([event])

@calum-chamberlain
Copy link
Member

The rendered docs page is here - I'm not sure what is going on with the syntax highlighting being all comment-coloured...

@tjnewton
Copy link
Contributor Author

tjnewton commented Aug 9, 2021

Wow that is a lot cleaner. Thanks for the tip!

@calum-chamberlain
Copy link
Member

@tjnewton are you okay if I pick this up and work on it? I think I'm going to try and put it on google collab or similar so that people can interact directly with it and will have to make some changes to make it work there.

@tjnewton
Copy link
Contributor Author

@tjnewton are you okay if I pick this up and work on it? I think I'm going to try and put it on google collab or similar so that people can interact directly with it and will have to make some changes to make it work there.

Certainly! Great idea.

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