6
\$\begingroup\$

This my first foray in F#, and I cannot think (yet) functional. I have implemented an interface for a discount curve and the implementation for a piecewise constant instant rate implementation.

I would like to have your opinion on how to make this more F#. Should I keep the second input of the constructor as an array or a sequence?

Also, as I pre-compute all possible values between the first point and the end_date, I could have done a tail recursion and lazily populate the cache, but I was wondering about efficiency: I did not have time to write the tail-recursion and do a timing comparison.

module public DiscountCurve = 
    type IDiscountCurve =
       abstract member Df : System.DateTime -> double

    type DfPieceWiseInstant(end_date:System.DateTime, points:(System.DateTime * double)[]) =
        let start_date = fst(points.[0])
        let end_date = end_date
        let cache : double array = Array.zeroCreate ((end_date - start_date).Days + 1)
        let points = points
        do
            cache.[0] <- 1.        
            for idx in { 0 .. (points.Length - 1) } do
                let cache_idx_start = max 1 (fst(points.[idx]) - start_date).Days
                let cache_idx_end = 
                    if idx+1 < points.Length then
                        (fst(points.[idx+1]) - start_date).Days - 1
                    else
                        (end_date - start_date).Days - 1
                let cache_idx_df = exp (-snd(points.[idx])/365.)
                for cache_date in seq { cache_idx_start .. cache_idx_end-1 } do
                    cache.[cache_date] <- cache.[cache_date-1] * cache_idx_df



        interface IDiscountCurve with 
            member this.Df(date) = 
                cache.[(date - start_date).Days]
\$\endgroup\$
  • \$\begingroup\$ Could you please post a sample input/output? \$\endgroup\$ – mjolka Aug 1 '14 at 23:43
2
\$\begingroup\$

I can't see how tail recursion comes into the picture exactly, but I would start with putting a stop watch around the do that populates the cache and checking how much work it actually needs to do, just to see whether you couldn't get away with what you have now. There's certainly some value in computing only the part of the curve you need, but I'd check how much that would gain you before making that code more complicated.

For starters, you certainly don't need let bindings for end_date and points. You already have them accessible anywhere within your type as they're constructor arguments. You can also drop the {} and seq {} in you for ... in ... do expressions.

A quick and simple way to delay populating of the cache until it's needed is to make it lazy.

let cache = 
     Lazy.Create <| fun () ->
        let cache = Array.zeroCreate ((end_date - start_date).Days + 1)
        (* the entirety of your 'do' goes here *)
        cache

interface IDiscountCurve with 
    member this.Df(date) = 
        cache.Value.[(date - start_date).Days]

This way it will only be populated once Df is called, and no work would be done on object's construction.

\$\endgroup\$
  • \$\begingroup\$ I don't see how would this kind of laziness be useful. Why would anyone ever create an instance of DfPieceWiseInstant without calling its only method? \$\endgroup\$ – svick Aug 3 '14 at 10:34
  • \$\begingroup\$ It makes a lot more sense than creating an instance of DfPieceWiseInstant, putting together that underlying array and only then not calling its only method, right? This is what laziness is about. And it makes sense if you want to create some discount curves upfront and then pass them around rather then hold on to those constructor arguments in case you'd want to create one. \$\endgroup\$ – scrwtp Aug 3 '14 at 18:06
1
\$\begingroup\$
module public DiscountCurve = 

public is the default access modifier for modules, so you don't need to write it explicitly.


type IDiscountCurve =

You can certainly define interfaces in F#, but I believe it's not very common. I would do it only if there was some benefit in doing it, which I don't see here (though the same applies to C#).


abstract member Df : System.DateTime -> double

Df sounds like a pretty bad name to me. Unless it's a term that anyone using the class will know for certain, I would try to find a better name.


 System.DateTime

Why do you keep repeating the namespace instead of opening it?


(fst(points.[idx]) - start_date).Days

This code is repeated, I would extract it into a helper function:

let dayNumber index =
    (fst points.[index] - start_date).Days

Note that you don't need parentheses around the parameter of fst.


if idx+1 < points.Length then

You're already using points.Length - 1 elsewhere, so I would keep using that to avoid confusion:

if idx < points.Length - 1 then

In general, your code is not very functional, it's object-oriented: you're using interfaces, objects, constructors, arrays and imperative code. Though I think that a big part of that is caused by the algorithm you're using and that would be hard to change.

One way to make it more functional would be to write it as a function that returns your Df function:

let dfPieceWiseInstant (end_date:DateTime) (points:(DateTime * double)[]) =
    // most of your code here

    fun date -> cache.[(date - start_date).Days]

This would be certainly more functional approach, though I'm not sure whether it's actually better.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.