-
Notifications
You must be signed in to change notification settings - Fork 2
[WIP] Improvement of testing routines #68
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
Conversation
22c2ba9
to
95b1f77
Compare
95b1f77
to
80bd058
Compare
80bd058
to
1adf4ce
Compare
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.
Thanks @AdelekeBankole This looks great, thank you!
We have just run these locally and everything is passing.
Just left a couple of comments for things to check or suggestions to make things a little clearer.
real(dp), dimension(num_cols, nrf) :: p_cam, p_int_cam | ||
real(dp), dimension(num_cols, nrf) :: var_cam | ||
real(dp), dimension(num_cols) :: var_cam_surface | ||
real(dp), dimension(num_cols) :: ps_cam | ||
real(dp), dimension(num_cols, nrf) :: var_sam, var_sam_exp |
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.
These used to be length 48, but now use nrf which is length 30.
Should they be size sam_sounding
instead of nrf
?
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.
I think they should be size nrf
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.
OK, is there a reason for reducing them from 48 to 30?
If we do not test the interpolation above the SAM grid here we should open an issue to say that this is potentially something to be added in future.
Co-authored-by: Jack Atkinson <109271713+jatkinson1000@users.noreply.github.com>
Co-authored-by: Jack Atkinson <109271713+jatkinson1000@users.noreply.github.com>
Co-authored-by: Jack Atkinson <109271713+jatkinson1000@users.noreply.github.com>
Co-authored-by: Jack Atkinson <109271713+jatkinson1000@users.noreply.github.com>
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.
Thanks @AdelekeBankole
I still have a query around the change of array size, but do not feel it is blocking provided we log this in the issues.
This is great stuff and makes the code more robust, thank you.
real(dp), dimension(num_cols, nrf) :: p_cam, p_int_cam | ||
real(dp), dimension(num_cols, nrf) :: var_cam | ||
real(dp), dimension(num_cols) :: var_cam_surface | ||
real(dp), dimension(num_cols) :: ps_cam | ||
real(dp), dimension(num_cols, nrf) :: var_sam, var_sam_exp |
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.
OK, is there a reason for reducing them from 48 to 30?
If we do not test the interpolation above the SAM grid here we should open an issue to say that this is potentially something to be added in future.
This PR will close #64