fixes issue #103 #104
fixes issue #103 #104
Conversation
…ventory instead of xseed parser
|
This can probably be squash and merged |
|
I'm going to setup the beta web services to use the new version of Lloyds ObsPy code. I'll then pull down this branch and test it before merging. Thanks for the fix. |
|
Thanks. Lloyd is discussing with Lion and Meggies as to why the public facing functions for combining responses were removed. Currently the only way responses are combined is within the NRL client so for now I just copied LLoyds function for that from the NRL client and modified it to work with RESP files as strings. So the generalized get_response function currently in ph5tostationxml could get added into the obspy code again if Lion and Meggies agree that there should be public functions fro combining responses. |
|
So, the RESP code has no been merged into the obspy master branch without adding the general combine response function. This code should be good to go then. |
|
I'll also update the install instructions and README to reflect that you now just clone the master branch of obspy |
|
Wait don't we need a general combine response function? I thought Lloyd was going to add it back in. |
|
Going to add a tiny bit of code to this though based on a few small changes in the final version of the RESP pr that was merged. I'll push this change in about an hour |
|
Do you know if there is a reason that they didn't add it back into the ObsPy code? It would be better to have it in their code. |
|
Lion decided that it is too specific to the NRL to be added in to the response module and that doesn't make sense in the NRL client either since you would be feeding it RESP files not from the NRL. He said the response combining has to make too many possible NRL use case only assumptions about the semantics of each stage of the response so it could break if someone were to use non standard RESP files. A little odd to me since RESP was created by IRIS specifically for this use case |
|
Oh man that really stinks. Now we have to maintain that code. I might ask them again on the PR. |
|
I think Lion wasn't 100% adverse to adding it to the nrl client even though it didn't really have anything to do with the nrl but didn't want it in the main response module |
|
Yeah we really don't want to maintain that code unless we absolutely have to. I know they also mentioned the possibility of adding it to a |
|
According to Lion we can do this in three lines: |
|
made those changes and testing. Looks to come out the same as the code from LLoyds nrl client. Looks like the recalculate_overall_sensitivity() function Lion added was built based on the nrl client combine_response function. Looks like the nrl client get_response function could be shortened a ton since a lot of what it does is recalculating the sensitivity but they can fix that later if they want. |
|
It should be the same. You can view that method here https://github.com/obspy/obspy/blob/master/obspy/core/inventory/response.py#L770-L843. Currently we are doing this work twice. |
ph5/clients/ph5tostationxml.py
Outdated
| dl_resp._get_overall_sensitivity_and_gain( | ||
| output=unit, frequency=frequency) | ||
| dl_resp.instrument_sensitivity.value = gain | ||
| dl_resp.instrument_sensitivity.frequency = freq |
There was a problem hiding this comment.
I think the code between lines 247-304 can be removed. This is already being done in recalculate_overall_sensitivity.
The following is all that is required:
dl_resp.response_stages.pop(0)
dl_resp.response_stages.insert(0, sensor_resp.response_stages[0])
dl_resp.recalculate_overall_sensitivity()
There was a problem hiding this comment.
Yeah I completely removed this function in the latest commit. It's all contained and cleaned up in get_response_inv now
There was a problem hiding this comment.
Cool I'm setting up the PH5WS Beta Web Services to use this version of the code. If everything still works I think it is safe to merge.
|
It should be good to go now and hopefully that code in obspy is now done with any major modifications |
|
The beta web services are running on this version and are working. I think we can merge. |
|
The live PH5 Web Services are now using this version in anticipation of the upcoming ObsPy 1.1.0 release. Can we tag this current PH5 version as v4.0.1? |
|
Yeah I htink we can tag this v4.0.1 and update the change log. I'm still investigating the memory usage in pph5toms and hope to have something for that early next week |
added get_response function to combine responses and now uses read_inventory instead of xseed parser