uaf-arctic-eco-modeling / dvm-dos-tem

A process based Dynamic Vegetation, Dynamic Organic Soil, Terrestrial Ecosystem Model.
MIT License
22 stars 24 forks source link

Simplify `ParentLayer.cpp` and `SnowLayer.cpp` #715

Closed dogukanteber closed 5 months ago

dogukanteber commented 6 months ago

Layer class has the following virtual methods:

virtual double getFrzThermCond()=0; // get frozen layer thermal conductivity
virtual double getUnfThermCond()=0; // get unfrozen layer thermal conductivity
virtual double getFrzVolHeatCapa()=0;// get frozen layer specific heat capcity
virtual double getUnfVolHeatCapa()=0;// get unfrozen layer specific heat capacity
virtual double getMixVolHeatCapa()=0;//Yuan

The implementation of some of these methods are same in ParentLayer.cpp and SnowLayer.cpp. We cannot merge the same methods in a single method. So, this PR attempts to explain why these methods have the same implementation. It also cleans up the method implementation.

Benjamin-Maglio commented 6 months ago

An example of a commenting structure for this method both in ParentLayer.cpp and SnowLayer.cpp could look something like the following. It may be wise to add the comment above the heat capacity and thermal conductivity functions respectively.

// get layer volumetric heat capacity (vhc)
// Note: though the following functions return the same value
//            they require different names to work inside TempUpdator.cpp
//            where these will be called by name and not by layer type. 
//            Hence they must match with the functions used in Layer.cpp
//            and SoilLayer.cpp.
// get frozen layer volumetric heat capacity
double ParentLayer::getFrzVolHeatCapa() {
  double vhc = vhcsolid ;
  return vhc;
};
// get unfrozen layer volumetric heat capacity
double ParentLayer::getUnfVolHeatCapa() {
  double vhc= vhcsolid ;
  return vhc;
};
// get mixed (partially frozen) layer volumetric heat capacity
double ParentLayer::getMixVolHeatCapa() {
  double vhc= vhcsolid ;
  return vhc;
};
dogukanteber commented 5 months ago

Here are the plots after this change:

result.pdf