-
Notifications
You must be signed in to change notification settings - Fork 118
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
refactor(tsp): Elevate CNC to generalized transport class #1404
refactor(tsp): Elevate CNC to generalized transport class #1404
Conversation
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.
Looks good, @emorway-usgs. We might want to think about renaming the TSP object to something other than CNC since this the second C refers to concentration. Maybe it could be something like SDV (specified dependent variable)? But we would still want the user to specify CNC for transport, and maybe something else for temperature?
@langevin-usgs, I like Regarding a potential alternative name for the GWE equivalent of |
541e356
to
fd031dd
Compare
Co-authored-by: langevin-usgs <[email protected]>
…e either constant conc (cnc) or constant temperature (cnt)
d3a3e97
to
a5a5ad2
Compare
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.
This looks good, but I wonder how this is going to play with the new Input Data Model (IDM) routines. We are still going to want separate DFN files for CNC and CNT, but I'm not sure how this is going to work IDM. Perhaps you should look at the constant head package for flow and see if you can implement IDM for CNC as part of this PR. Since IDM is fully implemented now for CHD, WEL, DRN, RIV, GHB, EVT, and RCH, it would make sense it is implemented now for CNC as well. Any thoughts?
@langevin-usgs, Thanks for bringing the IDM issue to my attention. Agreed on the separate .dfn files for CNC and CNT. From a documentation (mf6io.pdf) perspective alone it makes sense to have 2 separate .dfn files. Also agree that it makes sense to look into IDM support for SDV now. I'll have a look at what was done to CHD for supporting IDM and may need to get together with @mjreno for the finer points. |
Going in a different direction with CNC. Going to leave it as part of the GroundWaterTransport model for the time being. A parallel package, constant temperature ("CNT"), will be introduced with GWE. After CNT is introduced, can look into creating a new class that contains the common code among CHD, CNC, & CNT that each of these packages could inherit from. |
No description provided.