Fix PV registry descriptions (v8.0.1) #1089
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the Registry, the pv_vertex, pv_edge, and pv_cell variables are described as
"absolute vorticity/rho_zz", but the density component was previously removed
from the model code. This request updates the descriptions and units of these
variables to be consistent with the absolute vertical vorticity.
In Line 4816 of mpas_atm_time_integration.F (v7.3): "the original definition of
pv_edge had a factor of 1/density. We have removed that factor given that it was
not integral to any conservation property of the system"
Relative vertical vorticity is calculated beginning at Line 5606 (in v7.3) as:
do iVertex=vertexStart,vertexEnd
vorticity(1:nVertLevels,iVertex) = 0.0
do i=1,vertexDegree
iEdge = edgesOnVertex(i,iVertex)
s = edgesOnVertex_sign(i,iVertex) * dcEdge(iEdge)
!DIR$ IVDEP
do k=1,nVertLevels
vorticity(k,iVertex) = vorticity(k,iVertex) + s * u(k,iEdge)
end do
end do
!DIR$ IVDEP
do k=1,nVertLevels
vorticity(k,iVertex) = vorticity(k,iVertex) * invAreaTriangle(iVertex)
end do
end do
(with 'u' as the uncoupled normal component of horizontal velocity), and
planetary vorticity is added to get absolute vorticity at Line 5754:
pv_vertex(k,iVertex) = (fVertex(iVertex) + vorticity(k,iVertex))
Thus, pv_vertex is the absolute vertical vorticity at a vertex.
adff544 to
10f63ee
Compare
mgduda
added a commit
that referenced
this pull request
Jun 30, 2023
This merge fixes two issues with the field test functions in the test core. The first issue is with the field test functions in the test core. This arose when reading in threadErrs, but not setting it to zero. This means that the field tests will inheret the error codes from the halo exchange tests above. intent(out) also allows the compiler to do whatever it desires to the contents of that array as soon as it enters the function. This could lead to undefined behavior. The net result of this is that the field test codes are erroneously set to nonzero values while the tests are in fact passing. After setting all values to 0 before the tests run, the field tests now correctly show that they are passing. The second issue comes from the use of mpas_threading_get_thread_num to get the thread number that the tests are running on, but only running the tests when threadNum == 0. If one of the tests does actually fail, threadErrs( threadNum ) is updated to 1. Since fortran has 1 based arrays, this led to out of bounds errors in the event that a test actually did fail. This was fixed by adding 1 to threadNum, and checking that threadNum == 1. This ensures that we still update the first element in threadErrs if there is a failed test. * fix-field-tests: Fix out of bounds access in threadErrs on failure Fix MPAS field tests
dimomatt
pushed a commit
to dimomatt/MPAS-Model
that referenced
this pull request
Jun 30, 2023
PR MPAS-Dev#1089 fixed some of the issues in the tests so that they aren't included in a relatively unrealated PR. This merge will update the test routines and fix the two errors described in PR MPAS-Dev#1089 that caused the fields tests to arbitrarily fail.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the Registry, the pv_vertex, pv_edge, and pv_cell variables are described as "absolute vorticity/rho_zz", but the density component was previously removed from the model code. This request updates the descriptions and units of these variables to be consistent with the absolute vertical vorticity.
In Line 4816 of mpas_atm_time_integration.F (v7.3): "the original definition of pv_edge had a factor of 1/density. We have removed that factor given that it was not integral to any conservation property of the system"
Relative vertical vorticity is calculated beginning at Line 5606 (in v7.3) as:
(with 'u' as the uncoupled normal component of horizontal velocity), and planetary vorticity is added to get absolute vorticity at Line 5754:
pv_vertex(k,iVertex) = (fVertex(iVertex) + vorticity(k,iVertex))Thus, pv_vertex is the absolute vertical vorticity at a vertex.
This PR mirrors PR #986 but targets the
hotfix-v8.0.1branch.