Calculate average values for each day of the week for each Meter

Question

I have a program with these two methods. One method to import a set of data from a CSV within the given time period and store them in a dictionary. Here the data in CSV file is stored in following format with meter Id, Timestamp in UTC and value. For each meter there's approximately 2000 values.

"MeterId","Ts","Value"
10411,"2022-10-31T16:00:00.000Z","null"
10411,"2022-10-31T16:30:00.000Z",5150

My Read CSV method loads each line to a dictionary and loads each meter ID to a separate list. I passed the dictionary and list as reference since I'm using them for second method.

public static void ReadCSVData(DateTime mFromTime, DateTime mToTime, ref Dictionary<Tuple<string, DateTime>, float> mDictLines, ref List<string> mUniqueList)
    {

        Console.WriteLine(DateTime.Now.ToString() + " Reading CSV...");
        List<string> lstMeters = new List<string>();

        using (var reader = new StreamReader(gSourceFile))
        {
            while (!reader.EndOfStream)
            {
                //read single line
                var line = reader.ReadLine();
                var values = line.Split(',');

                DateTime timeValue;

                if (DateTime.TryParse(values[1].Trim('"'), out timeValue))
                {
                    if (timeValue >= mFromTime && timeValue < mToTime)
                    {
                        if (values[2].Trim('"') != "null")
                        {
                            float meterValue = float.Parse(values[2].Trim('"'), CultureInfo.InvariantCulture.NumberFormat);
                            DateTime meterDate = DateTime.Parse(values[1].Trim('"')).AddHours(8);

                            mDictLines.Add(Tuple.Create(values[0], meterDate), meterValue);
                            lstMeters.Add(values[0].Trim('"'));
                        }
                    }
                }
            }
        }
        mUniqueList.AddRange(lstMeters.Distinct());
    }

My second method get the dictionary and list as arguments and calculate average values for each day of the week for each meter.

public static void CalculateAverageValues(Dictionary<Tuple<string, DateTime>, float> mDictLines, List<string> mUniqueList)
    {
        List<object> arrDoc = new List<object>();

        IEnumerable<string> ieUnique = from itemUnique in mUniqueList select itemUnique;

        foreach (string meterIdUnique in ieUnique)
        {
            float mondayTot = 0;
            float tuesdayTot = 0;
            float wednesdayTot = 0;
            float thursdayTot = 0;
            float fridayTot = 0;
            float saturdayTot = 0;
            float sundayTot = 0;
            int mondayCount = 0;
            int tuesdayCount = 0;
            int wednesdayCount = 0;
            int thursdayCount = 0;
            int fridayCount = 0;
            int saturdayCount = 0;
            int sundayCount = 0;

            foreach (KeyValuePair<Tuple<string, DateTime>, float> dictItem in mDictLines)
            {
                Tuple<string, DateTime> lineId = dictItem.Key;

                if (meterIdUnique == lineId.Item1)
                {

                    switch (lineId.Item2.DayOfWeek)
                    {
                        case DayOfWeek.Monday:
                            mondayTot += dictItem.Value;
                            mondayCount++;
                            break;
                        case DayOfWeek.Tuesday:
                            tuesdayTot += dictItem.Value;
                            tuesdayCount++;
                            break;
                        case DayOfWeek.Wednesday:
                            wednesdayTot += dictItem.Value;
                            wednesdayCount++;
                            break;
                        case DayOfWeek.Thursday:
                            thursdayTot += dictItem.Value;
                            thursdayCount++;
                            break;
                        case DayOfWeek.Friday:
                            fridayTot += dictItem.Value;
                            fridayCount++;
                            break;
                        case DayOfWeek.Saturday:
                            saturdayTot += dictItem.Value;
                            saturdayCount++;
                            break;
                        case DayOfWeek.Sunday:
                            sundayTot += dictItem.Value;
                            sundayCount++;
                            break;
                    }
                }
            }
            float avgMonday = (mondayTot / mondayCount);
            float avgTuesday = (tuesdayTot / tuesdayCount);
            float avgWednesday = (wednesdayTot / wednesdayCount);
            float avgThursday = (thursdayTot / thursdayCount);
            float avgFriday = (fridayTot / fridayCount);
            float avgSaturday = (saturdayTot / saturdayCount);
            float avgSunday = (sundayTot / sundayCount);

            Console.WriteLine(DateTime.Now.ToString() + " Processed data for meter " + meterIdUnique);

            InsertDataToCSV(csv, meterIdUnique, avgMonday, avgTuesday, avgWednesday, avgThursday, avgFriday, avgSaturday, avgSunday);

        }
        File.WriteAllText(gResultFile, csv.ToString());
        Console.WriteLine(DateTime.Now.ToString() + " Process over");
    }

This code works fine but it takes approximately one minute to process data for 1500 meters. Is there a way I could speed it up?

Answer 1

Please note: the code has been reviewed in two parts.
Some modifications about the first part can be found under update #1 .

---

As I suggested in the comments you can take advantage of CsvHelper to parse csv for you.

I would suggest to split your ReadCSVData into two methods to embrace reusability:

• The first method (ReadCSV) should read and parse the csv.
• The second (FilterMeters) should perform the necessary filtering.

ReadCSV

The Meter class

class Meter
{
    public int MeterId { get; set; }
    public DateTime TimeStamp { get; set; }
    public int? MeterValue { get; set; }
}

• You might need to adjust the names of the fields and their data types for your needs

The ReadCSV method

static List<Meter> ReadCSV()
{
    using var fileReader = new StreamReader("sample.csv");
    using var csvReader = new CsvReader(fileReader, CultureInfo.InvariantCulture);
        
    csvReader.Context.RegisterClassMap<MeterMap>();
    csvReader.Context.TypeConverterOptionsCache.GetOptions<int?>().NullValues.Add("null");

    return csvReader.GetRecords<Meter>().ToList();
}

• It reads all the lines and try to parse them as Meter objects
• The field mapping is defined inside MeterMap (see next section)
• The null value handling for the MeterValue property is done via TypeConverterOptions

The MeterMap class

sealed class MeterMap : ClassMap<Meter>
{
    public MeterMap()
    {
        Map(m => m.MeterId).Name("MeterId");
        Map(m => m.TimeStamp).Name("Ts");
        Map(m => m.MeterValue).Name("Value");
    }
}

• This separation allows you to use different property names than the csv's column names

FilterMeters

static (Dictionary<(int, DateTime), int> groupedMeterValues, List<int> meterIds) FilterMeters(List<Meter> meters, DateTime fromTime, DateTime toTime)
{
    var filteredMeters = meters.Where(m => m.TimeStamp >= fromTime && m.TimeStamp < toTime && m.MeterValue.HasValue);
    return (filteredMeters.ToDictionary(m => (m.MeterId, m.TimeStamp), m => m.MeterValue.Value),
            filteredMeters.Select(m => m.MeterId).Distinct().ToList());
}

• You could use Tuples (as well as ValueTuples) as a return value as well
  • So, you don't need to pass parameters via ref
• The filtering can be easily done via Linq's Where
• The ToDictionary allows you to transform your data for your needs
  • BTW for this particular example we could also use here the Linq's GroupBy
• Linq's Distinct allows you to select only once the duplicate values

---

UPDATE #1: Suggestions for CalculateAverageValues

After started working with the CalculateAverageValues method I've just realized that we don't need the unique meter ids collection.

So, the above FilterMeters could be rewritten like this:

static Dictionary<(int, DateTime), int> FilterMeters(IEnumerable<Meter> meters, DateTime fromTime, DateTime toTime)
{
    return meters.Where(m => m.TimeStamp >= fromTime && m.TimeStamp < toTime && m.MeterValue.HasValue)
                 .ToDictionary(m => (m.MeterId, m.TimeStamp), m => m.MeterValue.Value);
}

If you would define a helper class/struct/record like bellow:

class AveragedMeter
{
    public int MeterId { get; set; }
    public DayOfWeek DayOfWeek { get; set; }
    public double AverageMeterValue { get; set; }
}

then the whole CalculateAverageValues could be as simple as this

public static List<AveragedMeter> CalculateAverageValues(Dictionary<(int MeterId, DateTime TimeStamp), int> groupedMeterValues)
{
    return groupedMeterValues
        .GroupBy(m => new { m.Key.MeterId, m.Key.TimeStamp.DayOfWeek })
        .Select(g => new AveragedMeter {
            MeterId = g.Key.MeterId,
            DayOfWeek = g.Key.DayOfWeek,
            AverageMeterValue = g.Average(m => m.Value) })
        .ToList();
}

As you can see here I removed those lines that are responsible for writing to csv.

Finally, let's connect the dots

var meters = ReadCSV();
var groupedMeterValues = FilterMeters(meters, from, till);
var averagedMeterValues = CalculateAverageValues(groupedMeterValues);
WriteAverageCSV(averagedMeterValues);

Answer 2

ReadCSVData

Peter has covered how best to replace this, but here's a review anyway:

• You have a lot of nested ifs, and that can make a lot of people uncomfortable, especially if they are looking at your code on a narrow screen. Consider performing negative checks, and jumping out if they fail (e.g. if you can't parse the time, you can either skip the row - as you are doing now - with continue, or perhaps consider throwing an exception, so that you can't lose data without knowing, or as per Peter's answer, record when the meter value is "null" and handle that behaviour elsewhere).
• Consider using a using 'statement' rather than block: again, people reading on narrow screens will thank you for limiting nesting, and the inclusion of the last line is unlikely to be an issue. Alternatively, consider for-eaching over File.ReadLines.
• Your parameters should not be ref parameters: you are not changing their value, only calling methods on the object to which they point. Using ref parameters will frighten your users, and may lead to hard to trace bugs.
• Following on, your method accumulates values in the lists: in theory you could add the same meter to the list of unique meters twice in two different calls. If you don't need to be accumulating entries like this, then consider using out parameters (or indeed only returning a single return value: as Peter Csala shows, it's nice to separate the reading of the CSV from the extra bits in ReadCSVData (e.g. filling the list of meter names can be done from the dictionary of meters so you only need to return one object)).
• Unless you are using an older version of C#, you can include the declaration of timeValue as part of the out argument (e.g. out var timeValue): this isn't a readability concern in my opinion, but something to be aware of it.
• Consider 'pre-trimming' the values array: using the non-trimmed number on the mDictLines.Add line looks like a bug, and it's just generally ugly. A proper CSV Parser will of course take care of this stuff for you. Something simple like line.Split('"').Select(s => s.Trim('"')).ToArray() is all you need, but since you are worried about perf, in-place modification with a for-loop may be more appealing and only similarly ugly.
• Further, consider giving all three values dedicated names up-front: var meter = values[0] etc.: will make the remaining code much more readable and gives you a place to perform any necessary transformations (e.g. trimming) exactly once.
• (warning: I have strong opinions about this that may not concord with others') Don't use Tuple or ValueTuple on public APIs (with the exception of using ValueTuple to return multiple parameters); put together a nominal type instead (like Peter's Meter): part of why record was added to the language was to make simple types like this as painless as possible.
• What is the .AddHours(8); about? That needs - at a minimum - a comment to explain it.
• .NET Naming Conventions would have you call the method ReadCsvData.
• Consider using double instead of float: unless you are talking about ludicrous amounts of data there is generally no reason to use float rather than double these days.

CalculateAverageValues

The main problem is all the duplication or the weekdays. The most direct resolution would be to use a dictionary (or even an array, if you dare, as DayOfWeek is an enum) to record the per-weekday information in another type, then the only duplication is when you project out the value for the call to InsertDataToCSV, but maybe that can be simplified also?

Other notes on CalculateAverageValues:

• You've built a Dictionary in ReadCSVData, but you're not using any of the useful behaviour of a dictionary (except implicitly throwing if there are duplicate entries): maybe just use a list? (e.g. what CsvHelper will give you).
• arrDoc is never used.
• IEnumerable<string> ieUnique = from itemUnique in mUniqueList select itemUnique doesn't do anything: just for-each over mUniqueList.
• What is the m prefix about? Does it mean meter? If so, say that: m on a parameter looks like something from C++ gone hideously wrong.
• Note that consuming Tuple here makes it hard to know what Item1 and Item2 are: don't use it.
• What is csv? This looks like it should be local, whatever it is.

Performance

Regarding performance, the most apparent issue is that you are looping over all the entries for each meter, making your runtime complexity quadratic in the number of meters, rather than linear. The easiest change that would address this would be to use GroupBy to split the data per-meter before doing anything else, e.g. var entriesByMeter = mDictLines.GroupBy(kv => kv.Key.Item1) and loop over the resulting groupings instead. Once you've fixed that, you should profile to determine where any performance problems are, so you can focus any efforts.

Peter has shown how you can use GroupBy again to simplify calculating the averages, and so replace most of this method as well.

Answer 3

You shouldn't need to read the entire CSV into memory just to calculate the average.

Suppose that the input might 2Tb, or 2 Pb even (lol). You have 7 days of the week; just count number of hours for each day and number of lines, that's it. 14 values. No lists, no nothing.

Answer 4

I am going to comment on some of your code and some of the flaws that I see in it. But that is addressed much further below. First, I am going to take a long-winded journey before I explain to you why one line of your code seems simple but produces the wrong result.

Brief Background on Process Controls Data

Your application falls under the category of Process Data related to Process Controls or Control Systems. As I mentioned in a comment, there are specialized databases for working with Process Data, be it a time-series database or what is called a Process Data Historian, sometimes referred to as Operational Historian.

Just a few of the thousands of possible data sources feeding data to the historian may be a DCS (Distributed Control System) or SCADA.

Industries using Process Data include, but are not limited to, Oil Refineries, Chemical companies, Pulp & Paper, Food & Drug, and Power (distribution & transmission).

Process Data has been around for many decades. Organizations that help set various standards are ISO, ISA (International Society of Automation), IEEE, and OPC Foundation, and a few others.

Process Data is known for having 3 properties associated with an instrument tag, as defined by the OPC Foundation founded in 1994:

OPC Data Access (for one tag)

1. a value,
2. the quality of the value, and
3. a timestamp

In your application, the Meter Id would serve as the Tag name, which one could argue is really property 0. You omit a quality, though a bad quality is inferred from a “null” value, a good value is inferred from a valid non-NaN floating point.

There are many common things associated with data historians and time-series databases. A few that may be relevant to the conversation are:

1. Timestamps are stored in UTC
2. Tags may be Stepped or not (called Interpolated)
3. Data compression may be at play
4. Favor time-weighted averages over event-weighted
5. Quality of data is at play, i.e. there can be bad values (re: your “null”)

All of this adds a fair amount of complexity over calculating a daily average. For starters, most process control engineers consider the “day” to be in the site’s local time. The timestamps are in UTC, and thanks to DST, there may be days that are 23-hours (e.g. during Spring-forward transition) or days that are 25-hours (e.g. during Fall-back transition).

For those new to Process Data, let’s show a difference of versus a Stepped (Red) Tag versus Not Stepped aka Interpolated (Blue) with the exact same recorded values in a historian:

Both tags have the same recorded values at the blue bullets but they are treated differently when they are shown in a trend. If I zoom in a section of the trend, and try to find an interpolated value near 11:00 AM (closest I could get to was 11:00:11 AM), the tags have different "interpolated" values. The solid blue dot is 243.559 on the blue slanted line and the red circle is 251.

The vertical gray line around 11:00:11 AM shows 2 values. Its important to realize these are NOT actual recorded values stored in a database. Rather they are manufactured or computed using a form of interpolation. If the tag is stepped, it carries the last recorded value over as is. If the tag is not stepped, an interpolated value is computed along the slanted blue line based on the 2 recorded smaller blue dots.

Back to daily averaging

It should be site local time, and there are good chances you may not have a recorded value exactly at midnight local time. That means you have to compute a value at midnight local time, and how that value is computed depends upon if that tag is Stepped or not(see previous image).

Most industries using process data may have massive amounts of data. I have seen power companies with 25+ millions of meters (tags) and they only record a daily value. I have seen others with 100K meters recording hourly values. That would be 876 million uncompressed recorded values in 1 year. Many will use compression. The simplest form of compression is to omit consecutive duplicate values along horizontal lines. For stepped, you would only need the first value. For interpolated, you would need the first and last, but you can omit any duplicates in-between. There is a more sophisticated form of compression, using a swinging-door algorithm, that compresses along slanted lines based on some significant value.

Because of compression and bad quality of data, you would most likely want to perform a time-weighted average. Consider a very contrived overly-simplified example: you have a reading of 4000 exactly at midnight and another reading of 5000 at 1:00 AM for the remainder of the day. The event-weighted average would be 4500, in that there are merely 2 recorded events. But the time-weighted average is 4958.333 because it was 4000 for a duration of 1 hour and for 23 hours it was 5000. The time-weighted calculation would be [ (1 hour X 4000) + (23 hours X 5000) ] / 24 hours.

YOUR SPECIFIC CODE

Offset erroneously performed twice, OR

Bad attempt to get back to UTC time

I think you are a little bit sloppy with your timestamp handling. I think its great that all timestamps in the CSV are in ISO 8601 format for UTC. Example: "2022-10-31T16:00:00.000Z".

But this one line of code gives you bad results:

DateTime meterDate = DateTime.Parse(values[2].Trim('"')).AddHours(8);

My guess is that since you have input a UTC timestamp that you want to offset it 8 hours to your local time. There are 2 issues with this. The first being that the parsing does not return DateTime with Kind of Utc. Rather, it reads the timestamp, sees that it is UTC, and then performs an internal offset to Local. Then you erroneously add more hours to offset it a second time.

You can test this is an intermediate window or break it down into smaller pieces.

DateTime meterDate = DateTime.Parse(values[2].Trim('"'));

Here meterDate has the correct time, but its Kind is not Utc despite passing in a UTC time string. Rather, its Kind is Local. It kind of goes against what one would think but you may want to consider:

DateTime meterDate = DateTime.Parse(values[2].Trim('"')).ToUniversalTime();

Now meterDate is truly with Kind Utc, which could now be offset BUT you really should not hardcode an offset using AddHours. If your time zone experiences DST, hardcoding an 8 hour offset is wrong half the year. Consider using NodaTime.org to make time zone handling effortless. Or you can hammer through with .NET’s DateTimeOffset and/or TimeZoneInfo classes. If used carefully, .NET is okay. NodaTime just makes it easier and I think the code is easier to read.

Maybe you AddHours(8) because you already noticed the time was offset to Local and you want to get it back to UTC. Again, you should not be performing an offset like that due to possible DST concerns. Even if you don’t observe DST, meterDate may superficially show the correct time but its Kind is Local, which is incorrect. The DateTime.ToUniversalTime() is would the correct thing to use as not only does the offset correctly during Standard or DST but the Kind would be also correctly be Utc.

Bad Data

As I mentioned earlier, a value of "null" implies a bad quality. Alternatively, you could replace "null" with "NaN" (case-SENSITIVE). This can be wrapped in quotes or not. If it is wrapped in quotes, you should trim it away like you do with dates. Side note: you can omit the quotes around both timestamp and value, since neither contain your Split separator (comma).

With NaN, you can parse it as a float without throwing an exception. It would return float.NaN. Now you can have a type of Quality property, let’s call it IsGood and make it be a Boolean.

public bool IsGood => !float.IsNaN(value);

And this matters because when performing a time-weighted average, the industry standard is to omit the times when it is bad. Another idea is to also include the percent good along with the calculated average, so that if the percent good is too low, you can reject the average.

Let’s pretend that a meter had 4 hours of NaN or bad data for a given day. Now the daily average is not 24 hours (or 23 or 25 on DST transition days), but rather would be 20 hours. That meter has 20 hours of good data for the day, for an average that would be 83.33 percent good (20/24).

I DISAGREE WITH YOUR DICTIONARY

I think this is wrong: Dictionary<Tuple<string, DateTime>, float>

Process Historians would have this as Dictionary<string, Tuple<DateTime, float>> , where the string key is the Tag or Meter name. This would modify Peter Casala’s very fine answer, but generally there are a minimum of 2 collections with process data:

1. Tags, definition of each instrument tag (in your case, meters)
2. Recorded Values, collection of tag, timestamp, value, and optionally, quality.

Your tag definition does not need to be named Tags, but I would expect at the very least to know if its Stepped or not. On an enterprise level, that meter may belong to a certain substation, in a given state or province, with perhaps the site’s lat-long if not the meter’s lat-long, and even a time zone.

public class Meter
{
    public int Id { get; set; }
    public string Name { get; set; }  // or maybe => $”Meter {Id}”;
    public bool IsStepped { get; set; } = true;
    // Optional
    public string Substation { get; set; }
    public string TimeZoneName { get; set; }
}

public class MeterData
{
    public int MeterId { get; set; }
    private DateTime _utcTime = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
    public DateTime Timestamp 
    {
        get => _utcTime;
        set
        {
            _utcTime = value.Kind == DateTimeKind.Unspecified
                        ? DateTime.SpecifyKind(value, DateTimeKind.Utc) 
                        : value.ToUniversalTime();
        }
    }
    public float Value { get; set; } = float.NaN;
    public bool IsGood => !float.IsNaN(Value);
}

At this point, you may not even need a dictionary, but if you did it would be Dictionary<Meter, List<MeterData>>. Or you may want to keep a separate HashSet<Meter> and a List<MeterData>. Of course, what you choose to do may mean adding other things to the classes, such as implementing IEquatable<Meter> and having GetHashCode() return Id.

You can go further down the rabbit hole by having Substation be a separate table of information, including the time zone info. I work on an enterprise level where I have tags in countries around the globe, so I have to lots of tables and classes to distinguish my data by culture, time zone, geolocation, etc.

If you have the luxury of being a single site application, you may assume everything is the same time zone, maybe by a configurable setting or maybe it is just Local time, in which case DateTIme.ToLocalTime() is your friend during averaging. And since you only have meters, you may assume ALL meters or either stepped or not, then you can omit the Meter class.

Wrapping it up

Wow, this became a really long answer. Again, Process Data is a huge, specialized topic with a LOT of special concerns. I’ve presented you with lots of alternatives and things to consider. At this stage, you have enough info:

1. To you know if each meter is Stepped or not,
2. To handle each meter’s time zone (or maybe its just Local),
3. and you know if each data value is good or not.

This gives you enough information for you to calculate a time-weighted average for each local day, where you:

1. Adjust for presence of bad data.
2. Determine midnight site local time for a start of a given day
3. Determine end of day, that is the next midnight site local time
4. Get a value at midnight site local time, be it an actual recorded value or interpolated based on whether tag (meter) is stepped or not.

There are hundreds of ways to get there from here, and you may decide to only use a fraction of what I discussed. At the very least, you should correct how meterDate is assigned, and find a better alternative to AddHours(8). If you do need to work with time zones, I strongly endorse NodaTime.

I started off talking about process historians. I will end with it as well. All of them will perform time-weighted averaging taking into account Stepped/Interpolated tags and accounting for bad data. Some of them allow for passing in a time zone so that they can correctly calculate a daily average from midnight-midnight for that time zone accounting for DST, and the occasional 23 or 25 hour day.