Skip to content
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

A potential bug in the 'u_baro' calculation of FALWA v.1.2.0 #108

Closed
sandrolubis opened this issue Jan 15, 2024 · 2 comments
Closed

A potential bug in the 'u_baro' calculation of FALWA v.1.2.0 #108

sandrolubis opened this issue Jan 15, 2024 · 2 comments
Assignees

Comments

@sandrolubis
Copy link

sandrolubis commented Jan 15, 2024

Hi all, after updating to FALWA v1.2.0, I spotted a potential bug in u_baro calculation where the output mirrors - i.e., SH = NH. I checked the 'interpolated_u' data; they are correct. Only 'u_baro' shows this issue. See the attached plot.

ubaro_falwa120

@chpolste
Copy link
Collaborator

I can reproduce the problem with QGFieldNHN22. QGFieldNH18 does not seem to be affected.

I think the loop in

do j = jstart,jend ! ### yet to be multiplied by cosine
ubaro(:,j) = ubaro(:,j)+uu(:,j+nd-1,k)*exp(-zk/h)*dc
urefbaro(j) = urefbaro(j)+uref(j-jb,k)*exp(-zk/h)*dc
enddo

needs to be adapted to the value of is_nhem as in the loop earlier in the function. Maybe remove

! Bounds of y-indices in N/SHem
if (is_nhem) then ! 5N and higher latitude
jstart = jb+1 ! 6
jend = nd
else
jstart = 1
jend = nd-jb ! nd - 5
endif

and replace everything with

    if (is_nhem) then
      do j = jstart,jend
        ubaro(:,j) = ubaro(:,j)+uu(:,j+nd-1,k)*exp(-zk/h)*dc
        urefbaro(j) = urefbaro(j)+uref(j-jb,k)*exp(-zk/h)*dc
      enddo
    else
      do j = jstart,jend
        ubaro(:,j) = ubaro(:,j)+uu(:,j,k)*exp(-zk/h)*dc
        urefbaro(j) = urefbaro(j)+uref(j,k)*exp(-zk/h)*dc
      enddo
    endif

@csyhuang, do you think this works with the indices? It seems to resolve the issue in my test notebook, but I'm worried about an off-by-one error...

@csyhuang
Copy link
Owner

csyhuang commented Jan 19, 2024

@sandrolubis Thanks for reporting the bug!
@chpolste Thanks for figuring out the cause of this issue! I think your solution works (maybe also define explicitly jstart and jend in the loop?)! Would you like to submit a bugfix PR (and update version number to v1.2.1)? If not, I can do it this weekend after work.
Thanks again 🙏 😊

@csyhuang csyhuang self-assigned this Jan 20, 2024
csyhuang added a commit that referenced this issue Jan 20, 2024
@csyhuang csyhuang mentioned this issue Jan 20, 2024
csyhuang added a commit that referenced this issue Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants