-
Notifications
You must be signed in to change notification settings - Fork 714
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
fix access violation crash #2137
base: develop
Are you sure you want to change the base?
Conversation
The regression test results:
|
@likeuclinux Could you separate out the edits in wrf_timeseries.F and start_em.F into their own PR? As it sounds like they are a separate issue from the not fully allocated dlg_bep array, it would help the review process if each PR was limited in scope to the exact issue being resolved. |
Can I just create another PR for both wrf_timeseries.F and start_em.F changes? |
great, I will do now |
Yes, sorry. That is what I meant. Those two files' edits appear to be related to improper deallocations which could be its own single PR |
I just create this PR for memory leak issue: |
I don't need re-create current PR 2137 for single file change relate to phys/module_bl_ysu.F ? |
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.
@likeuclinux This one has #2139 as a subset. Need to remove one. Do we want all these changes?
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.
Note that #2139 looks like a subset and if so can be closed.
This issue is fatal that cause wrf executable crash, while #2139 is just memory leak |
@weiwangncar This needs another review. It has #2137 as a subset and so needs to be merged after that one. |
…ofile after RETURN logic
|
||
!Allocate the arrays for wind components | ||
#if ( EM_CORE == 1 ) | ||
ALLOCATE ( earth_u_profile(grid%max_ts_level), earth_v_profile(grid%max_ts_level) ) |
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.
@likeuclinux Are the changes made in the PR for wrf_timeseries.F completely the same as in PR-2139?
fix access violation crash
TYPE: bug fix
KEYWORDS: crash, access violation error
SOURCE: Charlie Li, software developer from lakes environmental, Canada
DESCRIPTION OF CHANGES:
Problem:
wrf crashed for access violation frequently, check details in #1991
when namelist.input has
sf_urban_physics = 0
the /inc/allocs_5.F
IF(okay_to_alloc.AND.in_use_for_config(id,'dlg_bep').AND.(.NOT.grid%is_intermediate))THEN
will be false, then it would go to following branch:
ELSE
ALLOCATE(grid%dlg_bep(1,1,1),STAT=ierr)
if (ierr.ne.0) then
CALL wrf_error_fatal ( &
'frame/module_domain.f: Failed to allocate grid%dlg_bep(1,1,1). ')
endif
ENDIF
it only allocate (1,1,1) for memory, then it trigger crash in phys/module_bl_ysu.F
if(present(a_u_bep) .and. present(a_v_bep) .and. present(a_t_bep) .and. &
present(a_q_bep) .and. present(a_e_bep) .and. present(b_u_bep) .and. &
present(b_v_bep) .and. present(b_t_bep) .and. present(b_q_bep) .and. &
present(b_e_bep) .and. present(dlg_bep) .and. present(dl_u_bep) .and. &
present(sf_bep) .and. present(vl_bep) .and. present(frc_urb2d)) then
endif
the present() check in code won't help, since upper level
dyn_em\module_first_rk_step_part1.F
will always call pbl_driver with DLG_BEP=grid%dlg_bep, then it pass down to module_pbl_driver to module_bl_ysu
Solution:
the fix is actually using v4.5 logic like following:
if(present(a_u_bep) .and. present(a_v_bep) .and. present(a_t_bep) .and. &
present(a_q_bep) .and. present(a_e_bep) .and. present(b_u_bep) .and. &
present(b_v_bep) .and. present(b_t_bep) .and. present(b_q_bep) .and. &
present(b_e_bep) .and. present(dlg_bep) .and. present(dl_u_bep) .and. &
present(sf_bep) .and. present(vl_bep) .and. present(frc_urb2d)) then
endif
the flag_bep came from:
SELECT CASE(sf_urban_physics)
CASE (BEPSCHEME)
flag_bep=.true.
CASE (BEP_BEMSCHEME)
flag_bep=.true.
CASE DEFAULT
flag_bep=.false.
END SELECT
when namelist.inpu has sf_urban_physics = 0, flag_bep will be false,
thus the code to access array with (1,1,1) allocation won't execute
the change in wrf_timeseries.F and start_em.F are memory leak detected when use PGI option:
-g -O0 -traceback -Mchkptr -Mbounds -Ktrap=fp -Msave -tp=px
It will failed for: "0: ALLOCATE: array already allocated"
ISSUE: For use when this PR closes an issue.
Fixes #123
LIST OF MODIFIED FILES: list of changed files (use
git diff --name-status master
to get formatted list)phys/module_bl_ysu.F
share/wrf_timeseries.F
dyn_em/start_em.F
TESTS CONDUCTED:
RELEASE NOTE: Include a stand-alone message suitable for the inclusion in the minor and annual releases. A publication citation is appropriate.