Conversation
Summary of ChangesHello @joelridden, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to eliminate numerical artifacts that can occur during NZCVM calculations by standardizing the precision of the output depth. By rounding the result directly within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces rounding to 3 decimal places for the depth calculation in compute_z_threshold. This is a good change to ensure consistent output and prevent floating-point artifacts in downstream processing. The docstring has also been updated accordingly. I've added one suggestion to make the implementation more concise.
joelridden
requested review from
AndrewRidden-Harper,
lispandfound and
sungeunbae
sungeunbae
left a comment
sungeunbae
left a comment
There was a problem hiding this comment.
I'm ok with this change, just in case double check if you need to update the benchmark data
(The tolerence is 1e-5 in the test
)
root@mantle:/mnt/mantle_data/jenkins/nzvm/benchmarks/zvalues_test# cat zvalues_test.z
Station_Name,Z1.0(km),Z2.5(km),sigma
HNPS,0.195,5.675,0.300
TRMS,0.055,0.575,0.500
NSPS,0.105,6.675,0.300
MWZ,0.045,0.275,0.500
NCDS,0.305,6.525,0.300
MTHZ,0.055,2.275,0.500
WDPS,0.085,0.275,0.300
TSZ,0.045,0.275,0.500
NCHS,0.145,6.525,0.300
NAAS,0.255,6.475,0.300
WFSS,0.095,8.825,0.300
NGHS,0.115,6.675,0.300
OPSS,0.075,1.825,0.500
URZ,0.045,0.275,0.500
HAVS,0.055,0.275,0.500
DAVS,0.065,0.325,0.300
PWES,0.045,0.275,0.500
MOLS,0.065,0.375,0.500
WANS,0.035,0.275,0.500
INSS,0.055,0.325,0.500
NELS,0.395,0.425,0.300
NBSS,0.065,0.325,0.300
WDFS,0.305,4.475,0.300
NEWS,0.045,1.225,0.500
SOMS,0.095,0.325,0.300
VUWS,0.085,0.325,0.300
TEPS,0.125,0.375,0.300
LIRS,0.065,0.325,0.300
WTYS,0.085,0.375,0.300
FAIS,0.065,0.325,0.500
PGMS,0.275,0.325,0.300
KHLS,0.065,1.825,0.500
NNZ,0.045,0.325,0.500
UHSS,0.115,0.325,0.300
LRSS,0.205,0.325,0.300
MKBS,0.045,0.275,0.500
SEDS,0.695,2.125,0.300
PHHS,0.045,0.075,0.500
TRTS,0.055,0.325,0.300
KIKS,0.325,4.325,0.300
WEL,0.045,0.325,0.500
WDAS,0.075,0.275,0.300
MISS,0.065,0.325,0.300
UHCS,0.385,0.425,0.300
MKVS,0.055,0.275,0.500
MATS,0.055,2.125,0.500
TAIS,0.095,0.325,0.300
WEMS,0.155,0.325,0.300
BOWS,0.065,0.325,0.300
BWRS,0.065,0.275,0.500
Rounding had no effect on these, they are already different due to other testing issues. |
4 participants
Removes some of these weird artifacts that happen during the NZCVM calculation in the function so we don't need to do it in many other places such as NZGMDB etc.