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

bug fix: initialize lat/lon with zeros only for nested fire runs #1434

Merged
merged 2 commits into from
Mar 23, 2021
Merged

bug fix: initialize lat/lon with zeros only for nested fire runs #1434

merged 2 commits into from
Mar 23, 2021

Conversation

masih-e
Copy link
Contributor

@masih-e masih-e commented Mar 16, 2021

TYPE: bug fix

KEYWORDS: idealized fire nested domain simulation

SOURCE: Found by Masih Eghdami (NCAR), Pedro Jimenez (NCAR)

DESCRIPTION OF CHANGES:
Problem:
The atmospheric coordinates (lat/lons) are initialized with spatial coordinates, this will create a fatal error when the
model is run with nested domains. The error is because lat>90 in share/interp_fcn.F:

CALL wrf_error_fatal ( 'Nest over the pole, single input domain, longitudes will be wrong' )

Solution:
The lat/lon are initialized with zeros similar to LES idealized cases only if the model has 2 domains to prevent the error. If the model is run with 1 domain, the lat/lon are written with spatial coordinates allowing easier post-processing.

ISSUE: This PR addresses the discussion in #1412 with @pedro-jm and @janmandel.

LIST OF MODIFIED FILES:
module_initialize_fire.F

TESTS CONDUCTED:

  1. Yes, the nested idealized fire case runs without the error described above.
  2. They are passing.

dudhia
dudhia previously approved these changes Mar 22, 2021
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.

changes confined to fire code

@davegill
Copy link
Contributor

@masih-e @dudhia @janmandel @pedro-jm
Is it still not the case that a domain with 100 grid cells in the north-south direction would fail, whether it is a nest or the coarse grid?

@masih-e
Copy link
Contributor Author

masih-e commented Mar 22, 2021

The number of grid cells is not causing the error. It is the value of XLAT exceeding 85 that triggers the error. The error is called in an interpolation subroutine only for nested domains. Therefore, a single domain run should be fine regardless. I hope that answers the question @davegill

@davegill davegill changed the base branch from release-v4.2.3 to develop March 22, 2021 21:42
@davegill davegill dismissed dudhia’s stale review March 22, 2021 21:42

The base branch was changed.

 Changes to be committed:
	modified:   module_fr_fire_util.F
@masih-e masih-e requested review from brankokosovic, domingom, pedro-jm and a team as code owners March 22, 2021 21:49
@davegill
Copy link
Contributor

@masih-e @janmandel @pedro-jm
If the code is disabled for nested simulations (and things run OK), then why have the lat/lon assignments required for the single domain runs?

Basically, I am just bothered by the half-way solution.

  1. If no lat/lon assignment is required (which it does not seem to be), then we can just completely remove the coordinate setting step. Is that right?
  2. If a lat/lon assignment is a good idea, then do not leave the answer in meters. The center lat/lon of the domain is available. Assuming a really high resolution domain, a cartesian coordinate assumption is reasonable. Just set the center of the domain to be (cen lon, cen lat), and scale the rest of the values based on the grid distance.

@pedro-jm
Copy link
Contributor

@davegill

  1. You are correct. We can remove the coordinate setup and the code would run fine. The coordinate set up is useful for visualization.
  2. Note that the fix only affects idealized runs and for these runs there is no geolocation available for the fire case at this moment.
    Pedro.

@davegill
Copy link
Contributor

@pedro-jm @janmandel @masih-e
Folks,
This is an idealized initialization for WRF Fire. As Jimy said, this is entirely "in" WRF Fire. I defer to your preferences in this matter.

@davegill
Copy link
Contributor

@pedro-jm @janmandel @dudhia @weiwangncar @masih-e
Folks,

  1. The initialization is not required for the model.
  2. The initialization is used for visualization.
    Therefore we can leave the PR as it is.

Later, should anyone want to fix this, the projection information is defined in module_initialize_fire.F:

    CALL nl_set_iswater(1,0)
    CALL nl_set_cen_lat(1,40.)
    CALL nl_set_cen_lon(1,-105.)
    CALL nl_set_truelat1(1,0.)
    CALL nl_set_truelat2(1,0.)
    CALL nl_set_moad_cen_lat (1,0.)
    CALL nl_set_stand_lon (1,0.)
    CALL nl_set_pole_lon (1,0.)
    CALL nl_set_pole_lat (1,90.)
    CALL nl_set_map_proj(1,0)

This current routine would make a great way to initialize the lat lons for many of the 3d idealized cases.

@davegill davegill self-requested a review March 23, 2021 16:14
@davegill davegill merged commit 0d93150 into wrf-model:develop Mar 23, 2021
vlakshmanan-scala pushed a commit to scala-computing/WRF that referenced this pull request Apr 4, 2024
…-model#1434)

TYPE: bug fix

KEYWORDS: idealized fire nested domain simulation

SOURCE: Found by Masih Eghdami (NCAR), Pedro Jimenez (NCAR)

DESCRIPTION OF CHANGES:
Problem:
The atmospheric coordinates (lat/lons) are initialized with spatial coordinates, this will create a fatal error when the 
model is run with nested domains. The error is because lat>90 in share/interp_fcn.F:
```
CALL wrf_error_fatal ( 'Nest over the pole, single input domain, longitudes will be wrong' )
```

Solution:
The lat/lon are initialized with zeros similar to LES idealized cases only if the model has 2 domains to prevent the error. If the model is run with 1 domain, the lat/lon are written with spatial coordinates allowing easier post-processing.

ISSUE: This PR addresses the discussion in wrf-model#1412 with @pedro-jm and @janmandel.

LIST OF MODIFIED FILES: 
module_initialize_fire.F

TESTS CONDUCTED: 
1. Yes, the nested idealized fire case runs without the error described above.
2. They are passing.
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