QuakeMigrate
BUG: network fix
There was a small bug in the latest changes to allow for networks to be specified. This fixes this bug. Stay tuned for a more complete fix using the full NSLC codes
I fully agree that adding more information to the input station file is the ideal way to make QM more flexible in terms of the trace meta information. To me, the solution would be to extend (perhaps optionally) the columns that can be provided in the station input file e.g.:
Network | Code | Channels | Latitude | Longitude | Elevation
bla bla bla bla bla bla
However, we want to be able to retain the existing behaviour - if only Code (or Name as it is) is supplied, along with the geographical coordinates, then network and channels are assumed/decided by the information given to archive (in the case of channels)
One major benefit I can see of extending things in this way is that pick files will include all of the important information such that when imported into snuffler they are matched up with the corresponding traces. This doesn't currently really work (and it is a bit annoying!)
I can get ahead with stripping out the other updates from the previous PR. I hadn't realised that it held all the baggage from the previous rubbish ones. I'll do better this time.
My two cents worth is that I think it makes more sense to use trace ids as output from obspy and dealing with internally. This would make QM consistent with accepted naming conventions and include all the necessary information for snuffler.
I'll make the PR then you can take a look.
It would be best to start afresh from a new branch off development (so as to include changes made in the intervening time) and submit a new PR.
Again, how does changing things in this way impact previous results? We can construct the obspy-style IDs internally from the information provided in the above format. Doing this in the read_stations function and adding a column named Code would then minimise the amount of changes needed elsewhere.
Also, we have switched everything over to use Stream and Trace, rather than ararys of data, so we can use the .select() method of Stream to get what we need.
I think essentially my proposal is the same, but the reverse action is performed in read_stations. I think if we do choose to accept it in the trace_id style, we would only be able to accept this PR as an addition in a future version OR we would have to ensure backwards-compatibility is maintained. Otherwise, it would have to wait until we have a stable release with a version DOI that does it the old way.
One solution would be to supply the trace_id as a new column, e.g. Code or ID, rather than using the existing Name. Then we can do a check for if Code is supplied, and parse out all the information into Network, Name, Location, Channels. If it is not supplied, Name is treated as before, and the other parameters are assigned * (as you had in your read_stations method).
Yeh, to re-emphasise: the next update to development (in review atm) will significantly change how waveform data is handled internally. So we have an opportunity to take stock and decide what improvements we want to make (both what you have raised and looking to the future), how best to do them, and how to ensure backwards compatibility.
Just stripping out the irrelevant changes from your previous PR isn't going to leave us with a solution that satisfies all these requirements, and would have to be re-worked to make it compatible with this forthcoming change. So I would hold off for the moment and we can work out how to do it once and do it right - hopefully in a couple of days.
Sorry gents - I was already full steam ahead and wanted to at least send a PR that isn't formatted crap-ly. If you could take a look and share your opinions, I would be grateful. I'm not sure there are any problems with how the internal waveforms are referenced, but I'm not up-to-date with your future plans.
Tim
Tim Greenfield Leverhulme Early Career Research Fellow University of Cambridge [email protected]
From: Tom Winder [email protected] Sent: 30 October 2020 14:29 To: QuakeMigrate/QuakeMigrate [email protected] Cc: Tim Greenfield [email protected]; Assign [email protected] Subject: Re: [QuakeMigrate/QuakeMigrate] BUG: network fix (#96)
Yeh, to re-emphasise: the next update to development (in review atm) will significantly change how waveform data is handled internally. So we have an opportunity to take stock and decide what improvements we want to make (both what you have raised and looking to the future), how best to do them, and how to ensure backwards compatibility.
Just stripping out the irrelevant changes from your previous PR isn't going to leave us with a solution that satisfies all these requirements, and would have to be re-worked to make it compatible with this forthcoming change. So I would hold off for the moment and we can work out how to do it once and do it right - hopefully in a couple of days.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/QuakeMigrate/QuakeMigrate/issues/96#issuecomment-719585406, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALXCVON2ADXQOAX7NBBREO3SNLETPANCNFSM4RH6OLBA.