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

only memory leak related #2139

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

likeuclinux
Copy link

@likeuclinux likeuclinux commented Dec 5, 2024

TYPE: bug fix

KEYWORDS: Memory Leaks

SOURCE: Charlie Li - Software developer from lakes environmental Canada

DESCRIPTION OF CHANGES:
Problem:
Memory leaks are detected in wrf_timeseries.F and start_em.F when use PGI option:
-g -O0 -traceback -Mchkptr -Mbounds -Ktrap=fp -Msave -tp=px
It will failed for: "0: ALLOCATE: array already allocated"

  1. In dyn_em/start_em.F, dz8w is allocated, but not deallocated.
  2. In share/wrf_timeseries.F, two arrays, earth_u_profile and earth_v_profile, are allocated without being deallocated when time series is not computed.

Solution:
Calls to deallocate the array dz8w is added, and move the return statement before array allocation.

LIST OF MODIFIED FILES:
M share/wrf_timeseries.F
M dyn_em/start_em.F

TESTS CONDUCTED:
The Jenkins tests are all passing.

RELEASE NOTE: This PR fixed a memory leak related to arrays being allocated without being deallocated in start_em and time series calculation subroutine.

@likeuclinux likeuclinux requested review from a team as code owners December 5, 2024 20:16
@weiwangncar
Copy link
Collaborator

The regression test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

1 similar comment
@weiwangncar
Copy link
Collaborator

The regression test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

dudhia
dudhia previously approved these changes Jan 9, 2025
Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe for release branch

@dudhia dudhia changed the base branch from master to develop January 9, 2025 22:35
@dudhia dudhia dismissed their stale review January 9, 2025 22:35

The base branch was changed.

dudhia
dudhia previously approved these changes Jan 16, 2025
Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be committed before PR #2137 if we want to separate the changes, but it is also contained in that PR.

weiwangncar
weiwangncar previously approved these changes Jan 17, 2025
@weiwangncar weiwangncar self-requested a review January 17, 2025 01:29
@likeuclinux likeuclinux dismissed stale reviews from weiwangncar and dudhia via be5b815 January 28, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants