feat(prp): support coordinate transformations #1901
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@langevin-usgs this is generally according to our outline |
wpbonelli
force-pushed
the
prt-local
branch
3 times, most recently
from
f7b8c52 to
0cb9b66
Compare
|
This should be ready for a look when @aprovost-usgs is back |
There was a problem hiding this comment.
I know there may be more on work on this, but just wanted to mention one comment about the release notes. I'm good with whatever you and @aprovost-usgs decide on the coordinate options.
4 tasks
wpbonelli
force-pushed
the
prt-local
branch
2 times, most recently
from
16ad318 to
7318671
Compare
jdhughes-dev
pushed a commit
to jdhughes-dev/modflow6
that referenced
this pull request
Nov 12, 2024
…MODFLOW-ORG#2055) Carved out of MODFLOW-ORG#1901 for tidiness. Split TrackDataModule into TrackFileModule and TrackControlModule. Also clean up some comments, and pass fmi down into cell/subcell tracking methods (not always needed, but best to be consistent).
8 tasks
wpbonelli
added a commit
that referenced
this pull request
Jul 30, 2025
While it's on the user to provide both release coordinates and cell IDs, for some time we have checked at release time that release coordinates fall within the specified cell. Alternative ways to specify release points are in the works in #1901 e.g. local coordinates, which would remove the redundancy, but so long as release points are in global coordinates and we don't do cell identification ourselves, the coordinate checks are a performance bottleneck when particle count is high. Add a COORDINATE_CHECK_METHOD option to allow opting out. Checks on (by default) with eager, trust me I know what I'm doing mode with none. Later, consider adding a lazy mode where validation happens at tracking time, this might save some time if we can reuse terms of other calculations for the checks, but contradicts "fail early" philosophy so should probably not become default. Need to test and get some performance numbers. If in future we did start computing cell IDs (e.g. with something fast like the celltree algorithm) this option could just be ignored when cell IDs are not provided by the user. So one could decide whether to do it in preprocessing or let PRT handle it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PRT requires particle release points (except for z via
LOCAL_Z) specified in the model coordinate system, and also reports pathlines in the model coordinate system. This PR enables input coordinate transformations with keyword options for the particle release point package.With this change, points may be specified as
LOCAL_XYand/orLOCAL_Z— only structured gridsLOCAL_XY_OFFSET— structured or unstructured gridsDEV_GLOBAL_XY— transform release points to model coordinates using grid georeference informationChecklist of items for pull request
fprettify