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

fix(gwe-est): add support for energy decay in the solid phase #2155

Merged
merged 18 commits into from
Jan 31, 2025

Conversation

emorway-usgs
Copy link
Contributor

Support for specifying energy decay in the solid phase was missing despite it being described in the Supplemental Technical Information document. The best I can figure is that during development it was initially skipped over with the intention of coming back to it - but then was forgotten about.

autotest/test_gwe_decay01.py Outdated Show resolved Hide resolved
autotest/test_gwe_decay01.py Outdated Show resolved Hide resolved
autotest/test_gwe_decay01.py Outdated Show resolved Hide resolved
doc/mf6io/mf6ivar/dfn/gwe-est.dfn Outdated Show resolved Hide resolved
src/Model/GroundWaterEnergy/gwe-est.f90 Show resolved Hide resolved
src/Model/GroundWaterEnergy/gwe-est.f90 Show resolved Hide resolved
src/Model/GroundWaterEnergy/gwe-est.f90 Show resolved Hide resolved
src/Model/GroundWaterEnergy/gwe-est.f90 Show resolved Hide resolved
Copy link
Contributor

@aprovost-usgs aprovost-usgs left a comment

Choose a reason for hiding this comment

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

just some minor comments for your consideration

autotest/test_gwe_decay02.py Outdated Show resolved Hide resolved
doc/mf6io/mf6ivar/dfn/gwe-est.dfn Outdated Show resolved Hide resolved
doc/mf6io/mf6ivar/dfn/gwe-est.dfn Outdated Show resolved Hide resolved
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, only 2 suggestions to consider.

  1. Would seem nice to use enumerations for idcy and idcysrc.
  2. Should we rename "water" to "aqueous" to be consistent elsewhere?

src/Model/GroundWaterEnergy/gwe-est.f90 Outdated Show resolved Hide resolved
src/Model/GroundWaterEnergy/gwe-est.f90 Show resolved Hide resolved
@aprovost-usgs
Copy link
Contributor

@langevin-usgs I like your suggestion to be consistent, but the situation is a little muddy for me with respect to "aqueous" vs "water". In GWT, aqueous is used throughout. That makes sense because it's concerned with dissolved species -- "solutions" -- and "aqueous" is a common descriptor for that. But in GWE we're dealing with thermal energy. If it was just pure water being heated and cooled, "aqueous" would seem out of place. But in general it can be an aqueous solution that's being heat and cooled, so referring to it as the "aqueous phase" makes sense from that standpoint. That said, it seems to put a lot of (too much?) emphasis on the composition of the water. Another consideration is that we've already gone down the "water" path with properties in GWE, like DENSITY_WATER and HEAT_CAPACITY_WATER. That's a lot of hand wringing over a small point but just wanted to offer those perspectives

@langevin-usgs
Copy link
Contributor

@langevin-usgs I like your suggestion to be consistent, but the situation is a little muddy for me with respect to "aqueous" vs "water". In GWT, aqueous is used throughout. That makes sense because it's concerned with dissolved species -- "solutions" -- and "aqueous" is a common descriptor for that. But in GWE we're dealing with thermal energy. If it was just pure water being heated and cooled, "aqueous" would seem out of place. But in general it can be an aqueous solution that's being heat and cooled, so referring to it as the "aqueous phase" makes sense from that standpoint. That said, it seems to put a lot of (too much?) emphasis on the composition of the water. Another consideration is that we've already gone down the "water" path with properties in GWE, like DENSITY_WATER and HEAT_CAPACITY_WATER. That's a lot of hand wringing over a small point but just wanted to offer those perspectives

Okay, sounds good. Just wanted to make sure we weren't missing an opportunity to be consistent. Based on your comment and seeing that "aqueous" is often used in the context of water as a solvent, it might be best to defer this for now and maybe think on how to come up with clear terminology.

@emorway-usgs emorway-usgs merged commit 8414eb6 into MODFLOW-USGS:develop Jan 31, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants