LDMX-Software / pflib

Library and tool for interacting with polarfires.
https://ldmx-software.github.io/pflib/
4 stars 0 forks source link

Correct argument to PolarfirePacket::roc #81

Closed EinarElen closed 2 years ago

EinarElen commented 2 years ago

When looking a bit at @jmuse13's https://github.com/LDMX-Software/pflib/pull/71 PR, I noticed that his invocation of PolarfirePacket::roc differs from what was originally used in the charge injection code.

In tot_tune

          for (int i=0; i<nsamples; i++) {
            csv_out << ',' << data.sample(i).roc(half).get_adc(ichan);
          }

While earlier, we were doing

 for (int i=0; i<nsamples; i++) {
        csv_out << ',' << data.sample(i).roc(ilink).get_adc(ichan);
      }

In the tot_tune version, we are indexing by the ROC half (0 or 1), while in the earlier version we were indexing by ELINK number.

Looking at the TU for PolarfirePacket, it doesn't seem to agree between the header and source version about the name of the parameter

// Header  
 RocPacket roc(int iroc) const;
private:
  int offset_to_elink(int iroc) const;
// Source 
RocPacket PolarfirePacket::roc(int ilink) const {
  int offset=offset_to_elink(ilink);
  if (offset<0) return RocPacket(0,0);
  else return RocPacket(data_+offset,length_for_elink(ilink));
}

int PolarfirePacket::offset_to_elink(int ilink) const {
  if (ilink<0 || ilink>=nlinks()) return -1;
  int offset=2+((nlinks()+3)/4); // header words
  for (int i=0; i<ilink; i++)
    offset+=length_for_elink(i);
  return offset;
}

So... which version is correct? I'm hoping that the TOT tuning version is wrong because otherwise we have a big problem

tomeichlersmith commented 2 years ago

I know we just had a conversation about this but I took one more second to look and the indexing by ELINK is correct in terms of the specifications for the DAQ data format. I think we can patch this by updating the header to use the correct variables names (and perhaps add a bit of documentation too!).

The reason behind this confusion is annoying. In the DAQ data format and pflib, there is constant jumbling of calling ROCs links and links ROCs. This substitution has led to a lot of confusion. The good news for @jmuse13 is that using the half (0 - 1) is valid for the UMN setup since we only have two links (0 and 1).

EinarElen commented 2 years ago

Really good!