Embed Oudin's Fortran code

The PREMHYCE project need a faster version of the PE_Oudin() function.

The vectorization of the code is not satisfactory, it would be preferable to integrate the Fortran version of the code. I think that is a good idea to keep the R version of the code to facilitate the comprehension by the users. In this case we can add an argument to switch from the R code to the Fortran code.

added FORTRAN R SUGGESTION labels

created merge request !14 (merged) to address this issue

mentioned in merge request !14 (merged)

Oh yes, we need it !

I can do it if it helps.

@olivier.delaigue ?

mentioned in commit c9036a46

mentioned in commit b9d55e29

mentioned in commit 1e6cbf40

closed via merge request !14 (merged)

mentioned in commit d34d6c66

NB: the usage of the function is not the same if RunFortran = TRUE or FALSE

• FALSE: Lat can only be an atomic vector.
• TRUE: Lat can be an atomic vector or a vector of the same length as Temp

Indeed. For more clarity you could perhaps add a boolean to indicate if the calculation is done for several basins and check that it can only be activated with the RunFortran option.

reopened

@francois.bourgin The way time series of different stations are put one after the other is not satisfying (low user readibility) and does not comply with the way other airGR functionnalities are implemented.

I understand that you did that for CPU time reasons. Could you detail where the gain was? From calling less often the R function? From calling less often the frun function? The last Fortran function, which actually calculates PE, is anyway called for any time step of any basin.

I consider using data.frames or lists as input of the R code instead.

It just feels easier and faster to deal with gridded meteorological data. I don't really see the problem, the users don't have to use the new functionality if they don't need it, the default behaviour can be the same as before with a boolean value to indicate if the calculation is done for several locations.

Ok I better see your point now.

So we actually have 4 options:

• performing lapply or sapply on the current R function (not efficient according to @francois.bourgin when you want to deal with gridded data on a single time step, possibly because of all checks); this solution is actually already available to users
• the one proposed by @francois.bourgin (option allowing all grid points put as a time series and LAT proposed as a vector of same length)
• proposing that LAT and Temp (as inputs of the R function) have a spatial dimension, through lists or data.frames and
  • changing internally in the R function the data to vectors to run the Fortran on a single 'time series'
  • proposing an apply internally in the R function

Thinking out loud, I may also see a further option: a separate function for treating spatial data, with an option to agregate PE so that it can be directly inserted in other airGR functions.

@olivier.delaigue we need to brainstorm

I cleant the code proposed by @francois.bourgin and @olivier.delaigue , allowing for the implementation of @francois.bourgin option.

Changed all ET and ETP and PET to PE, indentation improved, everything in doubleprecision in Fortran, conversion to radian only done in the R code, headers improved. Please check.

frun_PE_gt.f90

PE_Oudin_gt.R

@antoine.pelletier spotted a couple of errors in the codes. Attached are modified versions of the two files.

frun_PE.f90

PE_Oudin.R

changed milestone to %v1.6.10

created merge request !19 (merged) to address this issue

mentioned in merge request !19 (merged)

mentioned in commit 52cf5081

mentioned in commit d6f27bc0

mentioned in commit 2a524d43

mentioned in commit 1d087445

mentioned in commit 5107b586

closed via merge request !19 (merged)

reopened

added Doing label

closed

reopened

See my comments in d6f27bc0

This is true, my mistake. Correct code attached.

PE_Oudin.R

mentioned in commit 7d0ed682

closed

marked this issue as related to #134 (closed)