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

Remove unused variables #49

Merged
merged 26 commits into from
May 19, 2015
Merged

Remove unused variables #49

merged 26 commits into from
May 19, 2015

Conversation

bartnijssen
Copy link
Collaborator

Removal of unused variables cuts down on compiler warnings. That way we can figure out which compiler warnings are meaningful. Many of the unused variables have to do with associate statements that were introduced by Martyn. Others are simply never used.

Note that the variables inside the (associate ...) statement do
not need to be defined in the enclosing function.
This includes unused dummy arguments in some function calls. Hence the update in systemSolv.f90 (groundwatr call)
This includes unused dummy argument ix_snowInterception in vegSWavRad_muster()
This includes unused dummy arguments in some function calls. Hence the update in systemSolv.f90.
@bartnijssen
Copy link
Collaborator Author

This pull request is complete. There are unused variables left in a few files:

  • soilLiqFlx.f90
  • coupled_em.f90
  • layerMerge.f90
  • layerDivide.f90
  • systemSolv.f90

But these need to be gone through carefully. If you are fine with the changes here, just go ahead and merge

! compute the Kersten number (-)
!kerstenNum = log10( (mLayerVolFracIce(iLayer) + mLayerVolFracLiq(iLayer))/theta_sat ) + 1._dp
! ...and, compute the thermal conductivity
!mLayerThermalC(iLayer) = kerstenNum*lambda_wet + (1._dp - kerstenNum)*lambda_drysoil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Opening up a new issue for an alternative method to compute thermal conductivity of soils

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But the point is that in the code that is distributed we should either implement it fully (as an option) or leave it out. You can have many branches in your own repo @ martynpclark/summa with work in progress, but that should not be part of the release summa version. My concern is that this quickly becomes intractable. People assume that they can uncomment the code and that it works, which may not be the case. Git does not care if you have 100's of branches.

I know git is frustrating in the beginning, but now that I am finally getting the hang of it, I think that is exactly how we should do it. Keep in mind that once we open the repo, our own repos will also be accessible to others, so we can always point collaborators to our own branches that may have special features. By necessity, the released version of SUMMA will always be a little behind the cutting edge (but it will be clean and stable).

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. I get it.

As an alternative can we open up a new issue and include the commented code
as part of the issue description?

Cheers
M.
On May 18, 2015 4:53 PM, "Bart Nijssen" notifications@github.com wrote:

In build/source/engine/diagn_evar.f90
#49 (comment):

@@ -244,12 +215,6 @@ subroutine diagn_evar(&
! iLayer, mLayerVolFracIce(iLayer), mLayerVolFracLiq(iLayer), mLayerThermalC(iLayer)

 endif
  • ! compute the thermal conductivity of the wet material (W m-1)
  • !lambda_wet = lambda_wetsoil**(1._dp - theta_sat) * lambda_watertheta_sat * lambda_ice(theta_sat - mLayerVolFracLiq(iLayer))
  • ! compute the Kersten number (-)
  • !kerstenNum = log10( (mLayerVolFracIce(iLayer) + mLayerVolFracLiq(iLayer))/theta_sat ) + 1._dp
  • ! ...and, compute the thermal conductivity
  • !mLayerThermalC(iLayer) = kerstenNum_lambda_wet + (1._dp - kerstenNum)_lambda_drysoil

But the point is that in the code that is distributed we should either
implement it fully (as an option) or leave it out. You can have many
branches in your own repo @ martynpclark/summa with work in progress, but
that should not be part of the release summa version. My concern is that
this quickly becomes intractable. People assume that they can uncomment the
code and that it works, which may not be the case. Git does not care if you
have 100's of branches.

I know git is frustrating in the beginning, but now that I am finally
getting the hang of it, I think that is exactly how we should do it. Keep
in mind that once we open the repo, our own repos will also be accessible
to others, so we can always point collaborators to our own branches that
may have special features. By necessity, the released version of SUMMA will
always be a little behind the cutting edge (but it will be clean and
stable).


Reply to this email directly or view it on GitHub
https://github.com/UCAR/summa/pull/49/files#r30555197.

martynpclark added a commit that referenced this pull request May 19, 2015
@martynpclark martynpclark merged commit 111f470 into CH-Earth:develop May 19, 2015
@bartnijssen bartnijssen deleted the cleanup/unused_variables branch May 19, 2015 22:42
wknoben pushed a commit that referenced this pull request Nov 17, 2024
Updates to prevent potential segmentation errors in opSplitting.f90 and  build flags in CMakeLists.txt
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