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

feature(discretization): add get_polyverts routine to DIS/DISV #1444

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

wpbonelli
Copy link
Contributor

@wpbonelli wpbonelli commented Nov 14, 2023

For PRT to check release points against cell IDs, add a get_polyverts() routine to dis/disv to get cell vertices as a 2D array of coordinates. Not tested here since discretization types aren't trivial to setup for unit testing, but testing ongoing in PRT

Also

  • use compact docstrings in DiscretizationBase.f90, minor docstring fixes
  • make overridden routines consistent and remove unneeded statements in DisBaseType

* retrieve cell vertices as a 2D coordinate array
* for use by PRT
@wpbonelli wpbonelli marked this pull request as ready for review November 14, 2023 20:48
Copy link
Contributor

@langevin-usgs langevin-usgs left a comment

Choose a reason for hiding this comment

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

Looks good to me. I made a few minor comments for DIS that also apply to DISV as well.

src/Model/GroundWaterFlow/gwf3dis8.f90 Outdated Show resolved Hide resolved
end if

! allocate vertices array
allocate (polyverts(nverts, 2))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be allocated and accessed as polyverts(2, nverts) to be consistent with Fortran column-major ordering? Probably doesn't matter a whole lot, but it would make sense to be consistent with the rest of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched to column-major indexing for get_polyverts as well as for point_in_polygon and its tests

src/Model/GroundWaterFlow/gwf3dis8.f90 Outdated Show resolved Hide resolved
* rename wrap -> closed
* specify clockwise vertex order in get_polyverts docstring
* column-major indexing for point_in_polygon and get_polyverts
@wpbonelli wpbonelli merged commit 8bf2da3 into MODFLOW-USGS:develop Nov 15, 2023
15 checks passed
@wpbonelli wpbonelli deleted the polyverts branch November 15, 2023 17:28
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