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

Treat Triangles Embedded in the Stream Mesh #87

Closed
wants to merge 5 commits into from

Conversation

saubhagya-gatech
Copy link
Contributor

@saubhagya-gatech saubhagya-gatech commented Nov 9, 2024

Triangles with all three vertices on the stream mesh can cause issues for applications like stream network connectivity. They often can divert water from corridor through them and act like a braid. To deal with this a two-stage treatment is implemented, with user options to select the level of treatment. These options can be accessed through a optional flag treat_stream_triangles : {'None', 'moderate', 'strict'} in tessalate_river_aligned function:

  • moderate: This will deploy refinement function of the triangle library, where stream triangles are identified and passed on for additional refinement. This can take of most of the triangles, however, it is not an strict enforcement as triangulation algorithm will try to balance between multiple constraints and refinement like angles and cell sizes.
  • strict: This method will split the stream triangles that could not be refined with the moderate option, by providing additional point (mid point of the off-river-corridor edge) during triangulation.

See image below where a stream triangle was split; red markers depicts the original stream triangle.

stream_triangle_treatment

@saubhagya-gatech
Copy link
Contributor Author

saubhagya-gatech commented Nov 9, 2024

These two tests are failing, I wonder if these are due to some change on NHD data?

FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties - assert 97 == 94
FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties_prune - assert 50 == 49

Copy link
Collaborator

@ecoon ecoon left a comment

Choose a reason for hiding this comment

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

See some changes/simplifications...

@ecoon
Copy link
Collaborator

ecoon commented Nov 12, 2024

These two tests are failing, I wonder if these are due to some change on NHD data?

FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties - assert 97 == 94 FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties_prune - assert 50 == 49

I'm not sure why this would be the case -- CI should redownload the data, and the last commit is passing, so the data would have had to change since Rich's last commit. Maybe this needs to be rebased onto master and then would pass?

@saubhagya-gatech
Copy link
Contributor Author

These two tests are failing, I wonder if these are due to some change on NHD data?
FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties - assert 97 == 94 FAILED watershed_workflow/test/test_hilev.py::test_river_tree_properties_prune - assert 50 == 49

I'm not sure why this would be the case -- CI should redownload the data, and the last commit is passing, so the data would have had to change since Rich's last commit. Maybe this needs to be rebased onto master and then would pass?

Ah, the above test were failing on my local machine as I might have older NHD database for this HUC. The CI is failing at downloading Daymet.
ERROR: failed to solve: process "/bin/sh -c pytest --nbmake --nbmake-kernel=python3 examples/get_Daymet.ipynb" did not complete successfully: exit code: 1

- we often have triangles at the stream confluence that have two edges
  on the river corridor. That does not impact results for most
  applications, however, can be critical for network connectivity
  studies.

- this update provide a point at confluences that are likely to have
  such triangles; this point has to be honored during the triangulation.
@ecoon ecoon force-pushed the ssr/conditioning_update branch from 777034b to d2b9d65 Compare November 12, 2024 15:51
@ecoon
Copy link
Collaborator

ecoon commented Nov 12, 2024

You had some weird merging going on this this branch, so I streamlined the branch, rebased onto master, and ran yapf on the code.

I also simplified the code in triangle_split_points -- I think this should be no change from your previous implementation, but please check and make sure it still works for you. Calls like "intersects" can be expensive -- you don't really want to do things like any(p.intersects(she) for shp in list_of_shapes), and you definitely don't want to do that whole thing in a loop. See my reimplementation for a better strategy.

@ecoon
Copy link
Collaborator

ecoon commented Nov 12, 2024

Note that since I rebased, you'll have to:

git checkout ssr/conditioning_update
git fetch
git reset --hard origin/ssr/conditioning_update

rather than just git pull (which will give you an error now).

@saubhagya-gatech
Copy link
Contributor Author

This is ready for merge.

* function to create discharge regions

* minor improvements

* refinement based on polygons

* fix for nan-mean

* polygon-based refinement

* provided labeled sets for cells, just upstream of the discharge faces

these cells regions are to be provided as:

        <Parameter name="direction normalized flux relative to region" type="string" value="discharge_cell_region_name" />

* cleanup
@saubhagya-gatech saubhagya-gatech deleted the ssr/conditioning_update branch February 14, 2025 17:42
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

Successfully merging this pull request may close these issues.

2 participants