GEOPY-2701: Export best match models as Maxwell plates#342
GEOPY-2701: Export best match models as Maxwell plates#342domfournier wants to merge 20 commits intodevelopfrom
Conversation
…ed opposite to template direction
github-actions
bot
changed the title
MatthieuCMira
left a comment
MatthieuCMira
left a comment
There was a problem hiding this comment.
Some docstrings to have.
Moreover, I have the feeling the function "unique responsibility" rule is not quite respected ;-)
and a bunch of questions for my understanding
| ) | ||
| local_polar[local_polar[:, 1] >= 180, 0] *= -1 # Wrap azimuths | ||
|
|
||
| # Flip the line segment if the azimuth angle suggests the opposite direction |
There was a problem hiding this comment.
Why do we want to flip?
There are some edge cases there.
How are they affected?
There was a problem hiding this comment.
Good question - We want an interpolation operator that goes from simulated -> "real survey line".
The library of simulations is all done on lines going West to East (with some extra angles to accommodate deviation).
If the real survey happens to be flown in the opposite direction (E->W) then the ordering of the points has to be flipped such that the order of data is in the same direction.
There was a problem hiding this comment.
And if the lines are S-N?
There was a problem hiding this comment.
The azimuth of the segment is always forced to be at the strike angle (or 0). So it's really just a matter of ordering. S-N would stay the same, but N-S would get flipped
There was a problem hiding this comment.
general comment:
We spoke about function single responsibility.
I have the feeling functions here are doing a lot of different logic at the same time that could be break into smaller function for readability.
There was a problem hiding this comment.
I hear you. I did try to parse out some functionality to standalone methods.
Highlight specific ones if you can.
| def normalized_data(data: np.ndarray, threshold=5) -> np.ndarray: | ||
| """ | ||
| Return data from a property group with symlog scaling and zero mean. | ||
| Return data from a property group with symlog, zero mean and unit max normalization. |
There was a problem hiding this comment.
I guess you data have no high extreme values?
If they have, it's recommended to use std, isn't it?
There was a problem hiding this comment.
Not too worried about extremes since values are first logged. The idea was to remove any large constant, which the std wouldn't pick up right? Like if the data is [10.1, 10.2, 10.5], the standard dev would be too small to bring the data down.
But point taken, maybe the median would be safer.
MatthieuCMira
left a comment
MatthieuCMira
left a comment
There was a problem hiding this comment.
Seems good for me.
Few docstrings comments
| ) | ||
| local_polar[local_polar[:, 1] >= 180, 0] *= -1 # Wrap azimuths | ||
|
|
||
| # Flip the line segment if the azimuth angle suggests the opposite direction |
There was a problem hiding this comment.
And if the lines are S-N?
benk-mira
left a comment
benk-mira
left a comment
There was a problem hiding this comment.
I have nothing to add. Looks good!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #342 +/- ##
===========================================
+ Coverage 91.26% 91.28% +0.01%
===========================================
Files 120 120
Lines 6505 6551 +46
Branches 785 787 +2
===========================================
+ Hits 5937 5980 +43
- Misses 379 380 +1
- Partials 189 191 +2
🚀 New features to boost your workflow:
|
3 participants
GEOPY-2701 - Export best match models as Maxwell plates