-
Notifications
You must be signed in to change notification settings - Fork 132
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
Synthetics Bugs #152
base: develop
Are you sure you want to change the base?
Synthetics Bugs #152
Conversation
minor update to make_synthetic
Added 1 more alias for SGRC from some GOM wells.
…tions-drop-36 Remove Py 3.6 from CI and update actions
make `defaults` on global namespace
Update AUTHORS.md info
Update defaults.py
…rned by fill_betweenx
…en renamed append_curve
…nd-curve-bug Fixed a bug where lasio no longer has the method add_curve.
…ecated_access Fix/curve deprecated access
…d-hatch Modern build system using Importlib and hatch
…urve Fix plot 2d curve
…cation Fix Pandas deprecations
I've reviewed this in light of the newer updates and the synthetic issues remain. This should merge into the existing code without any issues I could identify. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great. I've just got two nit-picks about testing None for truthiness and the obligatory question "do you have unit tests?"
Frank - I've reviewed this based on your comments and updated the as_curve function to eliminate the Nones. Added a few conditionals to account for depths that may not be realistic. Also added a unit test for the as_curve. |
Matt
I had some time so I took a look at this. Turns out the problem was with the wavelet. At some point in the past, the input into the wavelet calculation was made in seconds rather than milliseconds but the ricker calculation expected ms. I changed the ricker calculation to accept seconds (which I believe matches the bruges version).
During testing, I noticed a bug on the as_curve function so I fixed that as well with an improvement over the handling of the log output interval. I also made a change in the make_synthetic where the user can add their chosen sonic and density mneonics rather than having to rename them to DT and RHOB (which remain the defaults).
Hope this meets with your approval.
Corey