0

I am currently implementing belief propagation for discrete variables.

The messages are functions. I need to be able to combine them using products and sums to produce new functions.

I currently have a basic implementation using delegates, but I want to know if there is a better way to handle this. I'm also concerned about how this will scale using delegates.

Here is an example of my implementation:

public Func<double, double> ProductFunc(List<Func<double, double>> messages)
{
    // Construct the product of the message functions
    Func<double, double> productFunc = delegate(double x)
    {
        double productValue = 1.0;
        foreach(Func<double, double> message in messages)
        {
            productValue *= message(x);
        }
        return productValue;
    };

    return productFunc;

}

Is there a more effective way to achieve this?

Servy
  • 202,030
  • 26
  • 332
  • 449
user124784
  • 896
  • 1
  • 13
  • 22
  • 2
    Benchmark your code and *find out* if you have a performance problem. If you do, then describe what, specifically, the code needs to do that it fails to do, and what actually happens. – Servy Nov 24 '15 at 21:39
  • I'm voting to close this question as off-topic because there is already a working solution; there is no problem to be solved. – Servy Nov 24 '15 at 21:40
  • I haven't completed the implementation yet. I'm happy to benchmark it and optimize myself later but was hoping any glaringly obvious errors might be pointed out by the community :) – user124784 Nov 24 '15 at 21:41
  • It does seem weird to me that you have a function (method) just to create and return a Func. I'm under the impression you'll prolly only ever need one instance of this Func<>, in which case just make a static func and initialize it somewhere IMO. – GEEF Nov 24 '15 at 21:42
  • 1
    That's not what SO is for. If you have a specific, well defined, and clear problem, you can get an answer for it. You *don't*. – Servy Nov 24 '15 at 21:43
  • @Servy - Fair enough. Sorry! GEEF - Thanks for pitching in but I think it's best not to open up a discussion given Servy's comments. – user124784 Nov 24 '15 at 21:45
  • @GEEF I see no indication here at all that you'd only ever need one instance of it. – Servy Nov 24 '15 at 21:46
  • @Servy well, what circumstance would you need like 10 of these delegates floating around? I can think up some, but none of them really make more sense than having one place to do the calculation, whether that be one method call or one Func instance stored in some class. – GEEF Nov 24 '15 at 21:48
  • @Servy you're right, there is no indication he would only want one, but the design itself implies he may want more than one. Odds are, he probably only wants one. – GEEF Nov 24 '15 at 21:49
  • @GEEF - There will be a number of objects which each have their own functions (Funcs) and I will need to combine these to get the function belonging to another object. – user124784 Nov 24 '15 at 21:51
  • @user124784 ah you know what, I didn't see that you're passing Funcs into that method itself. I don't know if it got edited in there or I just overlooked it, but I thought it was kicking out the same func result over and over. – GEEF Nov 24 '15 at 21:54
  • Probably more appropriate for http://codereview.stackexchange.com/. – Marius Bancila Nov 24 '15 at 21:56

1 Answers1

1

Does the code you have actually do what you want?

I ask, because the specification isn't very clear. The code you have captures a reference to a List<Func<double, double>> object, and returns a Func<double, double> delegate that will enumerate the list as it is at the time the delegate is invoked (as opposed to using the list as it is at the time your method was called).

Maybe that's really what you want. It's consistent with the deferred execution that is used throughout e.g. LINQ. But it does mean that the caller should either intend that changes to the list will change the evaluation of the returned delegate, or it must be very careful to not change the list.

If instead what you are trying to achieve is to capture the mathematical relationship present at the time of the call, you might rather the method look something like this:

public Func<double, double> ProductFunc(List<Func<double, double>> messages)
{
    Func<double, double> productFunc = x => 1.0;

    foreach (Func<double, double> message in messages)
    {
        Func<double, double> currentFunc = productFunc;

        productFunc = x => currentFunc(x) * message(x);
    }

    return productFunc;
}

It seems to me that either way is fine. It just depends on what behavior it is you actually want the code to have. There's not enough context in your question for me to know that.

I'm also concerned about how this will scale using delegates

It should scale just fine. You're already using delegates anyway. Composing them in either fashion isn't likely to cause undue performance issues, and in any case the code is correctly expressive as-is. If you do run into a specific performance problem such that the code fails to meet some objective, measurable performance goal, then you can look at adjusting the code to address that (and likely losing at least some expressiveness in the process).

Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • This is a fantastic answer that really answers my questions right now. I'm sorry that I didn't make the question very clear - I didn't want to hang on the details of belief propagation too much. Thanks for your time! – user124784 Nov 25 '15 at 05:08