-
Notifications
You must be signed in to change notification settings - Fork 10
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
Create different number of mesh nodes in x- and y-direction #21
Create different number of mesh nodes in x- and y-direction #21
Conversation
Note that as this builds upon #21 the diff includes also those changes at the moment, and is not very useful. Once #21 is merged it will be more useful. Meanwhile a diff for only this PR can be found at: joeloskarsson/weather-model-graphs@archetype_changes...joeloskarsson:weather-model-graphs:coord_wise_refinement |
This makes a lot of sense, and thank you for adding the example image too. It makes it really clear what the improvement is here. I will give this a proper review once #21 is in so that it easier to understand the changes that this PR uniquely introduces |
85406dc
to
5e61df1
Compare
@leifdenby Now #19 is merged, so the diff for this PR is readable 😄 |
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.
Perfect! Thanks for adding the tests too! 🌟
Describe your changes
Currently the 2d multirange graphs created for multiscale and hierarchical graphs are restricted to laying out mesh nodes in an$n \times n$ grid. This is suitable for forecasting areas that are roughly square, but a poor choice for rectangular areas with a large difference between it's height and width.
This PR updates the way multiscale and hierarchical graphs are created to compute the refinement (or equivalently the number of mesh nodes at each level) separately in the x- and y-directions. The result of this is probably best understood by an example:
Here a mesh graph is created for an area with$10 \times 30$ grid nodes. We are making a hierarchical graph, plotting only nodes. Grid nodes have level -1. Before this change the graph would look like this:
Note that for level 0 there are 8 mesh nodes created in each direction, although this is way too many for the y-direction. This results in very short distances within the mesh in the y-direction compared to the x-direction. The higher levels share the same problem.
After this change the same graph looks like this:
Now the distance between mesh nodes is roughly the same in the x- and y-direction. Note that with this change we can no longer fit 3 mesh levels. This is actually desirable behavior, as the height of this area is actually too small to properly fit 3 levels.
This change builds upon #19 , and should be merged after that.
Issue Link
This provides some potential answers to #2. If those outstanding questions do not require further discussion I think this can close #2.
Type of change
Checklist before requesting a review
pull
with--rebase
option if possible).Checklist for reviewers
Each PR comes with its own improvements and flaws. The reviewer should check the following:
Author checklist after completed review
reflecting type of change (add section where missing):
Checklist for assignee