first velocity correction according to Rueger

[KentWheeler]

The functions first_vel_params, and first_vel_corrn have been rewritten and the function part_h2o_vap_press has been added. This allows calculation of the first velocity correction using either psychrometer or hygrometer readings.

Test data shows that the calculated corection is different from the original GeodePy calculations. Therefore, the calculations have been validated by comparing results to those obtained with 'Leica Captivate TS/MS Simulator'

[harry093]

@KentWheeler thanks for putting this together and congrats on your first contribution! It's going to be a good improvement. Unfortunately, the PR failed testing. I don't think it's a big thing, just a couple of things to sort out. This is not your fault because, as a first time contributor, the tests don't run automatically for you, I had to approve them running. I don't think that should be the case with your next PR (or maybe even for this one from now). Full details can be found here:

https://github.com/GeoscienceAustralia/GeodePy/runs/7352898589?check_suite_focus=true

Here's the relevant details:

There is a TypError because one of the tests is missing a parameter (frequency) and another test fails with an AssertionError, meaning that part of the code isn't working the way it should be.

[KentWheeler]

Thanks for looking at this Craig, I can see how to make reply to you via GitHub, but I guess that is just my limited knowledge of the functions of the site.

I can't see anything missing in my code. It works with my checks. But it does remind me of when Josh told me to go ahead and modify this code. He thought it might fail some of these tests. \

I think it must fail because the original code (And therefore the checking) only uses a few parameters. It does not use frequency or unit length in any of its calculations. I had found some inhouse code that would use a n n_ref value of 1.00028 if these two parameters were not supplied. However, I think this value has just been determined by averaging the n_ref values supplied for all thei instruments in the Appendix of Ruegers book. I could not find a reliable reference for using that. If I use this value with the test.py values I get (param_c = 280.000000000058, param_d = 79.39323495020614) which are reasonable answers.

The original survey.py also uses temp, pressure and humidity in the calculation of the C and D parameters. This does not make sense because these are instrument parameters, somethimes specific as constants by the manufacturere and not temperure dependant)

I can explain the AssertionError, I can only guess that this is because the functions I modified require different parameters than the originals.

Kent

On Sat, Jul 16, 2022 at 8:00 AM Craig Harrison @.***> wrote:

@KentWheeler https://github.com/KentWheeler thanks for putting this together and congrats on your first contribution! It's going to be a good improvement. Unfortunately, the PR failed testing. I don't think it's a big thing, just a couple of things to sort out. This is not your fault because, as a first time contributor, the tests don't run automatically for you, I had to approve them running. I don't think that should be the case with your next PR (or maybe even for this one from now). Full details can be found here:

https://github.com/GeoscienceAustralia/GeodePy/runs/7352898589?check_suite_focus=true

Here's the relevant details: [image: image] https://user-images.githubusercontent.com/22949242/179325557-432ed2de-67f4-4a74-a372-2fc10f4d745e.png

There is a TypError because one of the tests is missing a parameter (frequency) and another test fails with an AssertionError, meaning that part of the code isn't working the way it should be.

— Reply to this email directly, view it on GitHub https://github.com/GeoscienceAustralia/GeodePy/pull/142#issuecomment-1186029888, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALT4RTVY2Z7EXHUJ6TGW6PLVUH3P7ANCNFSM53GFS4CA . You are receiving this because you were mentioned.Message ID: @.***>