SeisComP / common

SeisComP framework C++ libraries, Python wrappers and messaging
Other
8 stars 21 forks source link

[pickerview] sort stations by hypocentral distance instead of epicentral one #105

Closed luca-s closed 1 year ago

luca-s commented 1 year ago

This complete part of #80

Please note that only the sorting has been fixed. The maximum station distance selector still refers to epicenter distance. There are too many hassles to deal with if we want to change that behaviour too and I don't believe is required at all.

gempa-jabe commented 1 year ago

I am fine with the implementation if the depth issue is resolved. I think there is a default depth configured if the origin depth is not set.

luca-s commented 1 year ago

PR updated

luca-s commented 1 year ago

As I side note, I have thought of applying the same logic of this PR to the amplitudeview, that is to populate the ITEM_DISTANCE_INDEX value of RecordViewItem with the hypocentral distance instead of the epicentral one. That would result in the correct sorting of the stations by hypocentral distance.

However I cannot do that because in the amplitudeview the ITEM_DISTANCE_INDEX value of RecordViewItem is also used to store the epicentral distance to be passed to the magnitude processor (see here and here for example).

I am mentioning this because you might have an idea of a simple workaround that doesn't require a major code change.

gempa-dirk commented 1 year ago

@luca-s The solution is a sound approximation at short distance where straight rays may be assumed and it may be reasonable at local to regional distances as weöö. At larger distances a hypocentral distance cannot be estimated by the cartesian distance but should it be the length of a ray? On the other hand the current implementation assumes epicentral distance and vertical distance hence, it may not harm. At larger epicentral distances will be longer than cartesian hypocentral distances. This will totally change the sorting of the traces resulting in very uncommon travel-time curves.

Therefore, a clear definition should be given somewhere, e.g. in the glossary, what a hypocentral distance actually is in a particular context. Note, some applications do may consider cartesian distances.

luca-s commented 1 year ago

I understand your comment, but at larger distances, when my approximation breaks, the epicentral distance is so large that it doesn't matter the vertical one, so the sorting would still be correct.

That said , an improvement would be to consider the hypocentral distance only when the epicentral distance is smaller than a certain value. Even better, only when the vertical distance is greater than X% of the epicentral one. What about it?

gempa-dirk commented 1 year ago

I am not too much in favor of adding lots of assumptions but I suggest that a clear definition of the terms and the methods along with potential limitations should be added to the documentation, tool tips, eventually configuration, etc.

luca-s commented 1 year ago

Let's drop this PR then. This should be a simple fix transparent to the user. If it gets too complicated I don't like it.

I can personally live with my locally modified version of the code for the few projects I maintain, so I don't necessarily need this PR.

Just be aware that if more users start using seiscomp at local scale then #80 becomes relevant.