USEPA / Stormwater-Management-Model

Dynamic hydrology-hydraulic water quality simulation model
237 stars 175 forks source link

Constants precision nits #70

Open MitchHeineman opened 3 years ago

MitchHeineman commented 3 years ago

Very minor stuff: a few constants are applied inconsistently and/or could easily be resolved to higher precision. The gravity adjustment is less than 0.1%; the Manning's N coefficient adjustment is close to 0.3%

//consts.h

define GRAVITY 32.2 // add precision to 32.174

define SI_GRAVITY 9.81 // never used

define PHI 1.486 // resolve to 1.4859 or exact representation as 0.3048^(-1/3)

//lid.c LidProcs[j].surface.alpha =1.49 * sqrt(LidProcs[j].surface.surfSlope) / LidProcs[j].surface.roughness; // use PHI

  LidProcs[j].surface.alpha = 1.49 / LidProcs[j].surface.roughness * sqrt(LidProcs[j].surface.surfSlope);           // use PHI

  LidProcs[j].drainMat.alpha = 1.49 / LidProcs[j].drainMat.roughness * sqrt(LidProcs[j].surface.surfSlope);     // use PHI

//statsrpt.c if (UnitSystem == US) Vcf = 7.48 / 1.0e6; // resolve to 7.4805

//subcatch.c const double MCOEFF = 1.49; // use PHI

//transect.c else Transect[j].hradTbl[i] = pow(qSum * Nchannel / 1.49 / aSum, 1.5); // use PHI

//link.c aa = Conduit[k].beta / sqrt(32.2) pow(Link[j].xsect.yFull, 0.1666667) 0.3; // use GRAVITY

dickinsonre commented 3 years ago

Those 32.2 values have something to do with SWMM4 and SWMM3 - here is how it is defined in SWMM4

  GRVT                 = 32.2
  IF(METRIC.EQ.2) GRVT = 9.806
  IF(AMEN.EQ.0.0.AND.METRIC.EQ.1) AMEN = 12.566
  IF(AMEN.EQ.0.0.AND.METRIC.EQ.2) AMEN =  1.22
michaeltryby commented 1 year ago

@cbuahin SWMM users place a high value on consistency of results. This change is going to affect results for every model out in the wild. Seems like a lot for a patch release. I recommend holding off on this until the next minor update.

dickinsonre commented 1 year ago

Technical Detail:

The constant PHI PHI is defined as 1.486, although it could be resolved to 1.4859. Significance:

The friction slope is one of the two most dominant terms in the dynamic wave solution of hydraulic models. Potential Impact:

Even a slight alteration in the value of PHI PHI could have a significant impact on outfall flows. This is particularly relevant when considering that this term is used in every iteration, for every time step, and for all links in the model. Recommendation:

Careful consideration should be given when defining the value of PHI (testing) PHI as even small changes could result in noticeable differences in model outputs.

MitchHeineman commented 1 year ago

While I try to avoid excessive precision in reports, I like detail in my calcs. I suppose the ultimate change would be to change the code from frumpy US units to SI and conform with the rest of the world, whereby PHI would be relegated to the dustbin. But we seem to have bigger cultural fish to fry than metrification. Although NIST and NOAA are inching forward and retiring the US Survey foot (0.304801 m) in favor of the international foot (0.3048 m): https://oceanservice.noaa.gov/geodesy/international-foot.html

cbuahin commented 10 months ago

Since we have decided to go with a minor update release, I think it is good to tackle this issue now.

michaeltryby commented 1 month ago

Reopening this issue ...

Benchmarks are sensitive to numerical precision of constants