Beware Earth radius in single precision#326
Conversation
interfaces/physics_schemes_interface/source/kernel/bl_exp_kernel_mod.F90
Show resolved
Hide resolved
Adrian Lock (Adrian-Lock)
left a comment
There was a problem hiding this comment.
Thanks, lots of nice tidying up too.
MichaelWhitall
left a comment
There was a problem hiding this comment.
Thanks Ian, this looks fine to me, thanks lots for fixing this and reinstating the comorph_dev rose stem test :)
|
Hi, could I just check if this is ready for code review? |
Yes - I thought Adrian's approval on 6th March should have notified and requested the review from you (at least the bot seemed to change the status and request the review from you). Let us know if we've done something wrong and this didn't happen though! |
No problem, the email might have just got lost amongst all the others GitHub likes to send! I'll start looking at this today and sorry for the delay 😄 |
|
So, I've been through the code and am happy with the changes in principle. I'm having a little trouble getting the |
I strongly suspect that is due to it being the 1st of the month and RAID checks on the HPC causing everything to run a lot slower than usual. I would hope that it should be fine when things are back to normal tomorrow (it was obviously fine when I tested the all group). |
Ahhh, that makes a lot of sense, thank you! I'll run the test group tomorrow and hopefully all will be fine. |
Alistair Pirrie (mo-alistairp)
left a comment
There was a problem hiding this comment.
The troublesome test did indeed pass today. Happy to approve, Code Review: PASS
|
On testing this seems to have encountered an intermitent failure, being fixed in #381. Hence, this PR is being blocked for the meantime |
PR Summary
Sci/Tech Reviewer: Adrian Lock (@Adrian-Lock)
Code Reviewer: Alistair Pirrie (@mo-alistairp)
When represented at single precision, the radius from the centre of the Earth to any model level is only accurate to 0.5m. This is not sufficient accuracy for calculations in the explicit boundary layer scheme, which often calculate delta_z values based off this to higher precision.
To address this, we try to remove as many instances of the use of r_theta_levels and r_rho_levels as possible, focussing instead on height from the surface where possible. If the total distance needs to be retained, it is done so at double precision.
This PR addresses #282
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - spbl_fixes/run4
Suite Information
Task Information
✅ succeeded tasks - 1314
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review