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

grid not identical to model.grid #1501

Closed
francispoulin opened this issue Mar 23, 2021 · 6 comments
Closed

grid not identical to model.grid #1501

francispoulin opened this issue Mar 23, 2021 · 6 comments

Comments

@francispoulin
Copy link
Collaborator

@ali-ramadhan and I discovered today in developing fjp/flat-for-shallow-water-model that grid is set up with default halos but model.grid actually has the correct halos needed for the advection scheme. Note that the advection scheme is not specified until after the grid is created so given the current framework I think this is necessary. I realize this might be less than ideal but as long as you are focusing on the physcial domain, the two agree exactly.

Is this a concern?

@glwagner
Copy link
Member

I think we should change the defaults as suggested by #1245 so this is uncommon.

@francispoulin
Copy link
Collaborator Author

Good idea.

Following the discussion, and the fact that people seem to be using high order advection schemes like WENO5, I changed the defalt in this branch from 1 to 3. This can be revisited when it becomes a PR so I will close this issue.

@ali-ramadhan
Copy link
Member

Changing the default would make inflating halos less common. I still think it would be useful to print a warning/info when halos are inflated so the user knows grid and model.grid are different now.

@francispoulin francispoulin reopened this Mar 23, 2021
@francispoulin
Copy link
Collaborator Author

I'm happy to put a warning if people agree. What would we say exactly and where would we say it?

@ali-ramadhan
Copy link
Member

How about this?

@warn "Inflating model grid halo size to ($Hx, $Hy, $Hz) and recreating grid. " *
      "The model grid will be different from the input grid. To avoid this warning, " *
      "pass halo=($Hx, $Hy, $Hz) when constructing the grid."

@francispoulin
Copy link
Collaborator Author

How about this?

@warn "Inflating model grid halo size to ($Hx, $Hy, $Hz) and recreating grid. " *
      "The model grid will be different from the input grid. To avoid this warning, " *
      "pass halo=($Hx, $Hy, $Hz) when constructing the grid."

Sounds great! I have added it in my branch. I will close this issue and we cna modify this when the branch makes its way as a PR. Soon I hope!

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