Finding nearest float values corresponding to a data point (of three variables) in a table with multiple columns using C++

Question

I have a data file which contains floating point data arranged in 4 columns. Each row represent a specific data point. The first column(X) is made up of 100 different values but the values are repeating. Fortunately they are sorted in ascending order. For each distinct value in first column the second column(Y) has a different number of values which are sorted but repetative in nature. Finally, for each distinct value in second column there are a no. of different values in thrid column(Z) which are sorted and non-repetative. Data file can be viewed here

I have to find the nearest data point corresponding to a specific value(x,y,z) from first three column and write the value present in the forth column of that data point. Following is the working code for this problem:

#include <iostream>
#include <fstream>
#include <iomanip>
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <sstream>
#include <cstdio>
#include <cstdlib>
#include <vector>
using namespace std;
#define Nc 381443
struct data {   //Stores value of data points row-wise
float H;
float Ba;
float SO4;
float sup;
};

struct ioninfo    //To store value of a specific variable and count the no. of times it is repeating
{
  float val;
  int count;
};

double write_absolute(double value)
{
  if (value<0)
  {
    return -value;
  }
  else return value;

}

int find_H_ion(double x, vector<ioninfo> Hion)  //To find value nearest to the first(x) variable in first column
{
    vector<double> diff(100);
    for (int i = 0; i < 100; ++i)
    {
      diff[i]=write_absolute(x-Hion[i].val);
    }
    double min=diff[0];
    int index;
    for (int i = 1; i < 100; ++i)
    {
      if (diff[i]<min)
      {
        min=diff[i];
        index=i;
      }
    }
    return index;
}

int find_Ba_ion(double y,int index, vector<int> nH_Ba, vector<ioninfo> Baion) //To find value nearest to the second(y) variable in second column
{
    vector<double> diffBa(nH_Ba[index]);

    int temp=0;

    for (int i = 0; i < index; ++i)
    {
      temp=temp+nH_Ba[i];
    }

    for (int i = 0; i < nH_Ba[index]; ++i)
    {
      diffBa[i]=write_absolute(y-Baion[temp+i].val);
    }
    double miny=diffBa[0];
    int indexy;
    for (int i = 1; i < nH_Ba[index]; ++i)
    {
      if (diffBa[i]<miny)
      {
        miny=diffBa[i];
        indexy=i;
      }
    }
    return indexy;
}

 int find_SO4_ion(double z,int index,int indexy, vector<int> nH_Ba,vector<ioninfo> Hion, vector<ioninfo> Baion, vector<data> v)  //To find value nearest to the third(z) variable in third column
{
    int sumA=0,sumB=0;
    for (int i = 0; i < index; ++i)
    {
        sumA=sumA+Hion[i].count;
    }
      int temp=0;
    for (int i = 0; i < index; ++i)
    {
        temp=temp+nH_Ba[i];
    }
    for (int i = 0; i < (indexy); ++i)
    {
      sumB=sumB+Baion[temp+i].count;
    }

    vector<double> diffz(Baion[temp+indexy].count);
    int pos=0; 
    for (int i = (sumA+sumB); i < (sumA+sumB+Baion[temp+indexy].count); ++i)
    {
        diffz[pos]=write_absolute(z-v[i].SO4);
        pos++;
    }
      double minz=diffz[0];
      int indexz;
      for (int i = 0; i < Baion[temp+indexy].count; ++i)
      {
        if (diffz[i]<minz)
      {
        minz=diffz[i];
        indexz=i;
      }
      }
    return (sumA+sumB+indexz);
}


int main()
{

    vector<data> v(Nc);
    vector<ioninfo> Hion(100);

    ifstream fin( "data2.txt");
    double X,Y,Z,S;
    double X1;
    int Hc[101];
    X=0.0,Y=0.0,Z=0.0,X1=0.0;

    int count=0;
    int local_count=1;
    int i=0;
    //Reading data points from file
    for(string line; getline( fin, line ); )
    {
        sscanf (line.c_str(),"%lf %lf %lf %lf",&X,&Y,&Z,&S);
        v[i].H=X;
        v[i].Ba=Y;
        v[i].SO4=Z;
        v[i].sup=S;
        i++;
}
fin.close();

//Finding distinct values in first column and storing the no. of times they are repeated
int j=0;
for ( i = 0; i < Nc-1; ++i)
{
  if (v[i].H==v[i+1].H)
  {
    local_count++;
  }
  else
  {
    Hion[j].val=v[i].H;
    Hion[j].count=local_count;
    j++;
    local_count=1;
  }
}
    Hion[j].val=v[i].H;
    Hion[j].count=local_count;

//For each distinct value in first column there exist different no. of distinct values in second column 
// First counting the no. of distinct values in second column corresponding to each distinct value in first column
int total_count=0;
int k;
int bin=0;
vector<int> nH_Ba(100);
for ( i = 0; i < 100; ++i)
{
  local_count=1;
  k=0;
  for (j = 0; j < Hion[i].count; ++j)
  {
    if (v[total_count].Ba==v[total_count+1].Ba)
  {
    total_count++;
  }
  else
  {

    bin++;
    total_count++;
  }
  }
    nH_Ba[i]=bin;
    bin=0;

}

//Counting the total no. of distinct value in second column for the purpose of allocating memory in further steps
int sumBa=0;
for (int i = 0; i < 100; ++i)
{
  sumBa=sumBa+nH_Ba[i];
}

//Again storing the distict values and their count
vector<ioninfo> Baion(sumBa+1);
total_count=0;
k=0;
for ( i = 0; i < 100; ++i)
{
  local_count=1;
  for (j = 0; j < Hion[i].count; ++j)
  {
    if (v[total_count].Ba==v[total_count+1].Ba)
  {
    local_count++;
    total_count++;
  }
  else
  {
    Baion[k].val=v[total_count].Ba;
    Baion[k].count=local_count;
    k++;
    total_count++;
    local_count=1;
  }
  }
}
    Baion[k].val=v[total_count].Ba;
    Baion[k].count=local_count;


    // Values nearest to these data are to be searced
    double x=55.5556; //First column
    double y=0.05;    //Second column
    double z=0.024;   //Third column

    //Finding values step by step from first column to third column 
    //and then printing the value present in fourth column
    int index;
    index=find_H_ion(x,Hion);//First column
    int indexy;
    indexy=find_Ba_ion(y,index,nH_Ba,Baion);//Second column
    int indexz=find_SO4_ion(z,index,indexy,nH_Ba,Hion,Baion,v);//Third column

      cout<<"Index="<<indexz<<endl;
      cout<<"Supersaturation="<<v[indexz].sup<<endl;


}

Can I improve this code any more?

I am avoiding the use of multidimensional array.

Answer 1

I see some things that may help you improve your program.

Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code, X1 and count within main are set to a value, never used, and Hc is unused entirely. My compiler also tells me that. Your compiler is probably also smart enough to tell you that, if you ask it to do so.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It isn't necessarily wrong to use, but be aware of when you absolutely shouldn't do it (such as in header files).

Use <cmath> instead of <math.h>

The difference between the two forms is that the former defines things within the std:: namespace versus into the global namespace. Language lawyers have lots of fun with this, but for daily use I'd recommend using <cmath>. See this SO question for details.

Only #include necessary files

There is no good reason that this program should need both <stdio.h> and <cstdio> or <stdlib.h> and <cstdlib>. As with the previous point, just use the C++ versions if they're needed at all.

Use existing library functions

Your write_absolute seems to duplicate exactly, the function of std::abs() for no apparent benefit.

Fix array bounds problem

The program currently has the following few lines within main():

for (j = 0; j < Hion[i].count; ++j) {
    if (v[total_count].Ba == v[total_count + 1].Ba) {
        total_count++;
    } else {
        // etc.
    }

But this and the other similar line below that have a problem because there is nothing that restricts total_count to only point within the valid range of v. In fact, on my machine, this causes a segfault and crash. I think the simplest way to fix it might be this:

for (j = 0; j < Hion[i].count && total_count+1 < Nc; ++j) {

But I haven't thoroughly dissected the program enough to determine if this is a valid fix or not. There is a similar issue in both find_Ba_ion() and find_SO4_ion() because the index and indexy values are not range checked and do go out of range in the program.

Pass by const reference where practical

Each of your find_xx_ion() routines takes one or more vectors as arguments. Because you're passing them by value, each vector is being duplicated and destroyed each time the functions are called, which is a considerable wast of time and memory. In this case, that's easily fixed. Use a const reference instead. For example, the declaration for find_SO4_ion() would look like this:

int find_SO4_ion(double z, int index, int indexy, 
    const std::vector<int> &nH_Ba, 
    const std::vector<ioninfo> &Hion, 
    const std::vector<ioninfo> &Baion, 
    const std::vector<data> &v);

Avoid scanf if you can

There are so many well known problems with scanf that you're usually better off avoiding it. Because of the possibility of a buffer overrun, code using scanf should at least specify an input buffer width:

    scanf("%1004s", s);

Even better is to use io extractors. You can eliminate the loop and a number of temporary variables by doing this:

while (fin >> v[i].H >> v[i].Ba >> v[i].SO4 >> v[i].sup) {
    ++i;
}
fin.close();

Eliminate "magic numbers"

There are a few numbers in the code, such as 100 and 381443 that have a specific meaning in their particular context. By using named constants such as MaxNumberOfUniqueValues, the program becomes easier to read and maintain. For cases in which the constant only has sense with respect to a particular object, consider making that constant part of the object. In all cases in C++, prefer using a const int or constexpr int to a #define.

Think about the usage of the program

There are a couple of different possible ways this could be used, and it's not clear which one you have in mind. One is that you are going to read the data into memory from a file and then search it many times. The alternative usage pattern is that you might have many data files, but would only tend to search each one once for a particular set of numbers. Once you determine which is more likely to be the usage, you can optimize in a particular direction.

Use classes and objects

Right now you have a number of relatively complex and error-prone data structures and functions. They are not really tied together in any way, leading to some of the indexing errors I mentioned above. Better would be to use data classes to encapsulate behavior and data. For example, you have a struct named data. First, let's give it a decent name and then let's turn it into a class. I've called it Ion which may not be correct, but it's better than data which is not at all descriptive of what the thing actually contains. Also, I've made all of the data members private:

class Ion {
private:
    float H;
    float Ba;
    float SO4;
    float sup;

Now a constructor:

public:
    Ion(float h=0, float ba=0, float so4=0, float Sup=0) :
        H(h),
        Ba(ba),
        SO4(so4),
        sup(Sup)
    {}

Next, let's define an extractor and inserter for convenience:

    friend std::istream &operator>>(std::istream &in, Ion &ion) {
        return in >> ion.H >> ion.Ba >> ion.SO4 >> ion.sup;
    }
    friend std::ostream &operator<<(std::ostream &out, const Ion &ion) {
        return out << ion.H << '\t' << ion.Ba << '\t' <<  ion.SO4 << '\t' <<  ion.sup;
    }

Now some comparison functions:

    float diffH(const Ion &rhs) const { return std::abs(rhs.H-H); }
    float diffBa(const Ion &rhs) const { return std::abs(rhs.Ba-Ba); }
    float diffSO4(const Ion &rhs) const { return std::abs(rhs.SO4-SO4); }

And an accessor for the supersaturation point:

    float getSup const { return sup; }
};

Now the main program gets much simpler:

int main() {
    std::ifstream fin("data2.txt");
    std::vector<Ion> v;
    v.reserve(Nc);

    for (Ion ion; fin >> ion; ) {
        v.push_back(ion);
    }
    fin.close();

    // This is what we're looking for:
    Ion target{55.5556, 0.05, 0.024};
    // find first element with minimum difference in H
    const auto H_minBegin = std::min_element(v.cbegin(), v.cend(), 
        [&target](const Ion &a, const Ion &b){ 
            return a.diffH(target) < b.diffH(target); 
        });
    // find end of range with identical H
    const auto H_minEnd = std::find_if(H_minBegin, v.cend(), 
        [&H_minBegin](const Ion &a){ 
            return a.diffH(*H_minBegin) > 0; 
        });
    // starting from there, find first element with min diff in Ba
    const auto Ba_minBegin = std::min_element(H_minBegin, H_minEnd,
        [&target](const Ion &a, const Ion &b){
            return a.diffBa(target) < b.diffBa(target);
        });
    // find end of range with identical Ba
    const auto Ba_minEnd = std::find_if(Ba_minBegin, H_minEnd, 
        [&Ba_minBegin](const Ion &a){ 
            return a.diffBa(*Ba_minBegin) > 0; 
        });
    // starting from there, find first element with min diff in SO4
    const auto SO4_minBegin = std::min_element(Ba_minBegin, Ba_minEnd,
        [&target](const Ion &a, const Ion &b){
            return a.diffSO4(target) < b.diffSO4(target);
        });
    // find end of range with identical SO4
    const auto SO4_minEnd = std::find_if(SO4_minBegin, Ba_minEnd, 
        [&SO4_minBegin](const Ion &a){ 
            return a.diffSO4(*SO4_minBegin) > 0; 
        });

    std::cout << "H_minBegin = " << *H_minBegin << " at line " << H_minBegin-v.cbegin() << std::endl;
    std::cout << "H_minEnd = " << *H_minEnd << " at line " << H_minEnd-v.cbegin() << std::endl;
    std::cout << "Ba_minBegin = " << *Ba_minBegin << " at line " << Ba_minBegin-v.cbegin() << std::endl;
    std::cout << "Ba_minEnd = " << *Ba_minEnd << " at line " << Ba_minEnd-v.cbegin() << std::endl;
    std::cout << "SO4_minBegin = " << *SO4_minBegin << " at line " << SO4_minBegin-v.cbegin() << std::endl;
    std::cout << "SO4_minEnd = " << *SO4_minEnd << " at line " << SO4_minEnd-v.cbegin() << std::endl;
    for (auto it = SO4_minBegin; it != SO4_minEnd; ++it) {
        std::cout << *it << std::endl;
    }
}    

Sample output

H_minBegin = 55.5556    3.8162e-08  0.0199582   1.01226 at line 0
H_minEnd = 55.5622  3.8162e-08  0.0240629   1.004 at line 5286
Ba_minBegin = 55.5556   0.054003    3.41241e-08 1.05729 at line 4215
Ba_minEnd = 55.5556 0.0661129   3.41241e-08 1.08745 at line 4302
SO4_minBegin = 55.5556  0.054003    0.0240629   785.06 at line 4287
SO4_minEnd = 55.5556    0.054003    0.0290117   841.572 at line 4288
55.5556 0.054003    0.0240629   785.06

Alternative distance calculation

Right now, the calculation in the original code and the code above first searches H, then Ba, and then SO4 in that order. However an alternative approach might also be useful and would definitely be faster. That approach would be to calculate an absoute numerical distance between the target and each line. One way it might be done (I don't know the physical interpretation of your data, so please use care in adopting this suggestion!) would be like this: $$ d = \left|\frac{H_T - H_i}{H_T}\right| + \left|\frac{Ba_T - Ba_i}{Ba_T}\right|+\left|\frac{SO4_T - SO4_i}{SO4_T}\right|$$

If that is a valid distance measure, then the code can be simplified still futher. We can define a distance member function:

float dist(const Ion &rhs) const {
    return std::abs((H-rhs.H)/H) + std::abs((Ba-rhs.Ba)/Ba)
        + std::abs((SO4-rhs.SO4)/SO4);
}

Now main gets even simpler:

int main() {
    std::ifstream fin("data2.txt");
    std::vector<Ion> v;
    v.reserve(Nc);

    for (Ion ion; fin >> ion; ) {
        v.push_back(ion);
    }
    fin.close();

    // This is what we're looking for:
    Ion target{55.5556, 0.05, 0.024};

    const auto dist_minBegin = std::min_element(v.cbegin(), v.cend(),
        [&target](const Ion &a, const Ion &b){
            return target.dist(a) < target.dist(b);
        });
    const auto dist_minEnd = std::find_if(dist_minBegin, v.cend(), 
        [&dist_minBegin](const Ion &a){ 
            return a.dist(*dist_minBegin) > 0; 
        });
    std::cout << "dist_minBegin = " << *dist_minBegin << " at line " << dist_minBegin-v.cbegin() << std::endl;
    std::cout << "dist_minEnd = " << *dist_minEnd << " at line " << dist_minEnd-v.cbegin() << std::endl;
    for (auto it = dist_minBegin; it != dist_minEnd; ++it) {
        std::cout << *it << std::endl;
    }
}    

Updated sample output

dist_minBegin = 55.5556 0.054003    0.0240629   785.06 at line 4287
dist_minEnd = 55.5556   0.054003    0.0290117   841.572 at line 4288
55.5556 0.054003    0.0240629   785.06

Still simpler

If we only care about the first match and no others, the code gets even simpler:

int main() {
    std::ifstream fin("data2.txt");
    std::vector<Ion> v;
    v.reserve(Nc);

    for (Ion ion; fin >> ion; ) {
        v.push_back(ion);
    }
    fin.close();

    // This is what we're looking for:
    Ion target{55.5556, 0.05, 0.024};

    const auto dist_minBegin = std::min_element(v.cbegin(), v.cend(),
        [&target](const Ion &a, const Ion &b){
            return target.dist(a) < target.dist(b);
        });
    // if we only care about the first match, we're done!
    std::cout << "Index = " << dist_minBegin - v.cbegin() 
        << "\nSupersaturation = " << (*dist_minBegin).getSup() << std::endl;
}    

Simple output

Index = 4287
Supersaturation = 785.06