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

Bug fix/hyd cond #436

Closed
wants to merge 2 commits into from
Closed

Conversation

martynpclark
Copy link
Collaborator

The code to compute hydraulic conductivity in macropores is

 if(volFracLiq > theta_mp)then
  theta_e       = (volFracLiq - theta_mp) / (theta_sat - theta_mp)
  hydCondMP_liq = (satHydCond_ma - satHydCond_mi) * (theta_e**mpExp)
 else

which means that the hydraulic conductivity for macropores will be negative if the conductivity for macropores is greater than for micropores.

The total hydraulic conductivity is

scalarHydCond   = hydCond_noIce*iceImpedeFac + scalarHydCondMP

and the total conductivity can be negative if there is substantial ice impedance.

It is a good idea to require that the conductivity for macropores be greater than for micropores.

@arbennett
Copy link
Member

arbennett commented Dec 3, 2020

Could we simply set the conductivity to zero in this case? And maybe issue a warning?

@bartnijssen
Copy link
Collaborator

@arbennett : Which conductivity would be set to 0?

Wouldn't it be easier to set the k_macropore to k_soil if k_macropore < k_soil and issue a warning, just to make sure that the model continues. If we can modify the message to indicate what a user can do to fix this that may be helpful as well.

if(k_macropore(iLayer-nSnow) < k_soil(iLayer-nSnow))then
      k_macropore(iLayer-nSnow) = k_soil(iLayer-nSnow)
      message=trim(message)//"hydraulic conductivity for macropores is less than the hydraulic conductivity for micropores"
      err=20; return
     endif

@martynpclark
Copy link
Collaborator Author

@bartnijssen yes, this is a good idea. A better solution. It means that the model does not stop.

@arbennett
Copy link
Member

Yeah, I agree, @bartnijssen that sounds like a good solution.

@andywood
Copy link
Collaborator

andywood commented Dec 4, 2020 via email

@andywood
Copy link
Collaborator

andywood commented Dec 4, 2020 via email

Copy link
Member

@arbennett arbennett left a comment

Choose a reason for hiding this comment

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

It seems @andywood, @bartnijssen, and myself all would like to see the workaround so that this doesn't stop the simulation. I think what we really want to ensure is that at line 1031 of soilLiqFlxf.90:

scalarHydCond = max(hydCond_noIce*iceImpedeFac + scalarHydCondMP, 0.0_dp) # or maybe some tiny value

Though, doing the above is just treating a symptom. I remember when running into this previously I had seen very divergent matric head values, so that might be worth tracking down.

@martynpclark
Copy link
Collaborator Author

Implemented changes in a separate PR

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.

4 participants