1

I wrote this for a meteorology class. For some reason, the textboxes aren't updating on the GUI correctly for large numbers (large numbers of photons). The calculation completes, but the textboxes don't update.

I suspect the problem is with calling Invoke(), but I can't for the life of me see what's going wrong. I've tried using both Invoke() and BeginInvoke() with similar results.

Can anyone help figure out where I'm going wrong?

Thanks!

PS> Please forgive the global variables. Was planning on cleaning them up later...

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Threading;

namespace CloudTransmittance
{
public partial class Form1 : Form
{

    public Form1()
    {
        InitializeComponent();
    }


    private void buttonCalculate_Click(object sender, EventArgs e)
    {

        Thread t = new Thread(calculateModel);
        t.Start();


    }

    //number of photons that have gone to albedo, direct, or diffuse transmittance
    private ulong top = 0;
    private ulong direct = 0;
    private ulong diffuse = 0;
    private ulong absorbed = 0;
    private ulong failed = 0;
    private ulong photons = 0;


    private void calculateModel()
    {
        //model variables
        double theta = 0;
        double tauStar = 0;
        double omega = 0;
        double g = 0;
        photons = 0;

        //Get data from form
        theta = Convert.ToDouble(textBoxTheta.Text);
        tauStar = Convert.ToDouble(textBoxTau.Text);
        omega = Convert.ToDouble(textBoxOmega.Text);
        g = Convert.ToDouble(textBoxG.Text);
        photons = Convert.ToUInt64(textBoxPhotons.Text);

        //Clear the progress bar and set its limits

        this.progressBar1.BeginInvoke(
            (MethodInvoker)delegate()
            {
                this.progressBar1.Minimum = 0;
                this.progressBar1.Value = 0;
                this.progressBar1.Maximum = (int)photons;
                this.progressBar1.Step = 1;
            });

        //Clear the text boxes
        this.textBoxAlbedo.Invoke(
            (MethodInvoker)delegate()
             {
                  this.textBoxAlbedo.Text = "";
             });
        this.textBoxDirect.Invoke(
           (MethodInvoker)delegate()
           {
               this.textBoxDirect.Text = "";
           });
        this.textBoxDiffuse.Invoke(
           (MethodInvoker)delegate()
           {
               this.textBoxDiffuse.Text = "";
           });
        this.textBox1.Invoke(
           (MethodInvoker)delegate()
           {
               this.textBox1.Text = "";
           });
        this.textBox2.Invoke(
           (MethodInvoker)delegate()
           {
               this.textBox2.Text = "";
           });

        //convert theta to radians from degrees
        theta *= Math.PI / 180;

        //number of photons that have gone to albedo, direct, or diffuse transmittance
        top = 0;
        direct = 0;
        diffuse = 0;
        absorbed = 0;
        failed = 0;

        //Random number generator
        Random r = new Random();
        double randomValue = 0;

        int count = 1000; //number of iterations of the problem...
        double delta = 0.00001; //close enough to "1" for calculations, since C# random goes from [0, 1) instead of [0, 1]
        //Calculate transmittance

        for (ulong photonCount = 0; photonCount < photons; photonCount++)
        {

            bool scattered = false;
            double newTheta = theta; //needed for looping
            int i = 0; //counting variable used to prevent infinite looping
            for (i = 0; i < count; i++)
            {
                double length = calculateTauP(); //length of the photon's travel
                double newTau = calculateTau(newTheta, length);
                if (newTau < 0)
                {
                    top++; //photon has exited through the top
                    break; //move to the next photon
                }
                else if (newTau > tauStar)
                {
                    //exited through the bottom of the cloud
                    if (scattered == false)
                    {
                        //direct transmittance
                        direct++;
                    }
                    else
                    {
                        //diffuse transmittance
                        diffuse++;
                    }
                    break;
                }
                else
                {
                    //photon is either scattered or absorbed
                    randomValue = r.NextDouble();
                    if (randomValue >= omega)  // || ((omega == 1) && (randomValue >= (omega - delta)) )
                    {
                        //photon absorbed, no longer of interest
                        absorbed++;
                        break;
                    }
                    else
                    {
                        //photon scattered, determine direction
                        scattered = true;
                        newTheta = calculateNewAngle(newTau, newTheta, g, randomValue);
                    }
                }
            }
            if (i >= count)
            {
                failed++;
            }
            this.progressBar1.BeginInvoke(
            (MethodInvoker)delegate()
            {
                this.progressBar1.PerformStep();
            });
        }


        //Update Form values
        displayData();
    }

    private void displayData()
    {
        if (this.textBoxAlbedo.InvokeRequired)
        {
            this.textBoxAlbedo.Invoke(
            (MethodInvoker)delegate()
            {
                this.textBoxAlbedo.Text = ((double)top / (double)photons).ToString();
            });
        }
        else
        {
            textBoxAlbedo.Text = ((double)top / (double)photons).ToString();
        }
        if (this.textBoxDirect.InvokeRequired)
        {
            this.textBoxDirect.Invoke(
               (MethodInvoker)delegate()
               {
                   this.textBoxDirect.Text = ((double)direct / (double)photons).ToString();
               });
        }
        else
        {
            textBoxDirect.Text = ((double)direct / (double)photons).ToString();
        }
        if (this.textBoxDiffuse.InvokeRequired)
        {
            this.textBoxDiffuse.Invoke(
               (MethodInvoker)delegate()
               {
                   this.textBoxDiffuse.Text = ((double)diffuse / (double)photons).ToString();
               });
        }
        else
        {
            textBoxDiffuse.Text = ((double)diffuse / (double)photons).ToString();
        }
        if (this.textBox1.InvokeRequired)
        {
            this.textBox1.Invoke(
               (MethodInvoker)delegate()
               {
                   this.textBox1.Text = absorbed.ToString();
               });
        }
        else
        {
            textBox1.Text = absorbed.ToString();
        }
        if (this.textBox2.InvokeRequired)
        {
            this.textBox2.Invoke(
               (MethodInvoker)delegate()
               {
                   this.textBox2.Text = failed.ToString();
               });
        }
        else
        {
            textBox2.Text = failed.ToString();
        }

    }



    private double calculateNewAngle(double length, double angle, double g, double randomNumber)
    {
        double newAngle = 0;
        double cos = (1 / (2 * g)) * (1 + Math.Pow(g, 2) - Math.Pow(((1 - Math.Pow(g, 2)) / (1 + g * (2 * randomNumber - 1))), 2));
        newAngle += angle + cos;
        while (newAngle >= 2 * Math.PI)
        {
            newAngle -= 2 * Math.PI; //normalize the angle to 0 <= angle < 2PI
        }


        return newAngle;
    }

    private double calculateTauP()
    {
        Random r = new Random();
        double distance = -1 * Math.Log(1 - r.NextDouble());
        return distance;
    }

    private double calculateTau(double angle, double tauP)
    {
        double tau = tauP * Math.Cos(Math.PI/2 - angle);
        return tau;
    }
}

}

8
  • Good idea :) I tried that, but same results.
    – Maxthecat
    Apr 25, 2012 at 22:15
  • 1
    Try out localize a problem by using simplified version of your program just for the test, remove all progress bar related code, leave one textbox and update it each second, let's say hardcode following 1) textBox.Invoke(); 2) Thread.Sleep(1000); 3) textBox.Invoke() and see whether it works and put here simplified example
    – sll
    Apr 25, 2012 at 22:17
  • Ooh! Just noticed that with the statusbar code removed, the update code works as expected... I wonder if it's because it's calling the step function for the updatebar too many times (ie: if you call it 1,000,000 times, the messages for the textboxes are lost)
    – Maxthecat
    Apr 25, 2012 at 22:23
  • messages could not be lost, is Form auto-closed after operation has been done?
    – sll
    Apr 25, 2012 at 22:29
  • @user1250984: I wonder how long one iteration of your inner loop takes. If it's very short, you may want to consider throttling UI updates to 1-2 per second, perhaps using a Stopwatch and checking elapsed time in each iteration, rather than blindly trying to update the UI each time in the outer loop. Then perhaps set a breakpoint on a textbox update and check if it indeed is invoked...
    – Alan
    Apr 25, 2012 at 22:30

1 Answer 1

4

Stop using Invoke and BeginInvoke to update the UI. Despite what you may have been told it is not that great of solution. Actually, in situations like these where all you want to to do is update the UI with progress information it is probably the worst solution. Instead, have your worker thread publish its progress information to an immutable data structure that can be shared with the UI thread. Then have your UI thread poll for it on a reasonable interval using a System.Windows.Forms.Timer.

public class YourForm : Form
{

  private class ProgressInfo
  {
    public ProgressInfo(ulong top, ulong direct, ulong diffuse, ulong absorbed, ulong failed, ulong photons)
    {
      // Set properties here.
    }

    public ulong Top { get; private set; }
    public ulong Direct { get; private set; }
    public ulong Diffuse { get; private set; }
    public ulong Dbsorbed { get; private set; }
    public ulong Failed { get; private set; }
    public ulong Photons { get; private set; }
  }

  private volatile ProgressInfo progress = null;

  private void calculateModel()
  {
    for (ulong photonCount = 0; photonCount < photons; photonCount++)
    {
      // Do your calculations here.

      // Publish new progress information.
      progress = new ProgressInfo(/* ... */);
    }
  }

  private void UpdateTimer_Tick(object sender, EventArgs args)
  {
    // Get a local reference to the data structure.
    // This is all that is needed since ProgressInfo is immutable
    // and the member was marked as volatile.
    ProgressInfo local = progress;
    this.textBoxAlbedo.Text = ((double)local.Top / (double)local.Photons).ToString();
    this.textBoxDirect.Text = ((double)local.Direct / (double)local.Photons).ToString();
    this.textBoxDiffuse.Text = ((double)local.Diffuse / (double)local.Photons).ToString();
    this.textBox1.Text = local.Absorbed.ToString();
    this.textBox2.Text = local.Failed.ToString();
  }

Notice several things here.

  • The code is a lot easier to understand and follow.
  • The UI thread gets to decide when and how often its controls should be updated.
  • You get more throughput on the worker thread since it does not have to wait for a response from the UI as would occur with Invoke.

I rip on these Invoke and BeginInvoke solutions a lot because in many cases they are terrible solutions. Using a BackgroundWorker is a little better, but it still forces you into the push method of updating the UI (via the same marshaling techniques behind the scenes nonetheless). The pull method can be (and often is) a more elegant solution and usually more efficient.

1
  • 1
    Awesome answer! I agree, emphatically. This is a much more elegant way to implement the code. I'll be using this model in the future. Thank you! :)
    – Maxthecat
    Apr 26, 2012 at 21:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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