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

BUG: network fix #96

Open
tmgreenfield1101 opened this issue Sep 11, 2020 · 7 comments
Open

BUG: network fix #96

tmgreenfield1101 opened this issue Sep 11, 2020 · 7 comments
Assignees

Comments

@tmgreenfield1101
Copy link
Collaborator

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

@tmgreenfield1101 tmgreenfield1101 self-assigned this Sep 11, 2020
@tmgreenfield1101 tmgreenfield1101 linked a pull request Sep 11, 2020 that will close this issue
@hemmelig hemmelig mentioned this issue Oct 30, 2020
@hemmelig
Copy link
Member

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!)

@tmgreenfield1101
Copy link
Collaborator Author

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.

@hemmelig
Copy link
Member

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.

@hemmelig
Copy link
Member

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.

@hemmelig
Copy link
Member

hemmelig commented Oct 30, 2020

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).

@TomWinder
Copy link
Member

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.

@tmgreenfield1101
Copy link
Collaborator Author

tmgreenfield1101 commented Oct 30, 2020 via email

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 a pull request may close this issue.

3 participants