Photon-HDF5 / phconvert

Convert Beker&Hickl, PicoQuant and other formats to Photon-HDF5
http://photon-hdf5.github.io/phconvert
Other
16 stars 14 forks source link

added missing function process_t2records #39

Closed seb5g closed 5 years ago

seb5g commented 5 years ago

@tritemio I just added the process_t2records that was missing (gone during one of the cleaning/pushing) here is a T2 file that can be used as a test: https://drive.google.com/file/d/1vMROMeCmUO_JwTt10aM3CoJLp06CbSrx/view?usp=sharing

tritemio commented 5 years ago

Ok thanks, I added your data file to Figshare for easy download:

https://ndownloader.figshare.com/files/14850533

Questions/comments:

  1. Your file contains 'rtTimeHarp260PT2' records. Do you have a file for 'rtTimeHarp260NT2' which is also added in the code?

  2. I see similarities with the process_t3records function, can they merged in a single function? If not, can you briefly point out at the differences?

  3. Did you check that all the wording in the process_t2records docstring actually makes sense or it is a copy of the process_t2records docstring?

Things to do/fix before merge:

It should suffice to add:

def dataset5():
    fn = 'trace_T2_300s_1_coincidence.ptu'  # 'rtTimeHarp260PT2'
    fname = DATADIR + fn
    return fname

then add dataset5 to the fixture params. Finally you may need to add the record byte code 0x00010206: 'rtTimeHarp260PT2' in test_read_ptu_recordtype.

Thanks.

tritemio commented 5 years ago

Also, please rebase your patch off current master for easy testing. You first have to pull the latest commit into your fork, then add the new commit on top. Let me know if you have difficulties with this.

seb5g commented 5 years ago

Hello, to be honest I'm a bit at a loss with all the branches/pulling stuff. I don't have a 'rtTimeHarp260NT2' file as my hardware is of the P type. There are similarities between the 2 functions and they could be merged but the allocated bits in the records have different meaning.

The docstrings makes sense.

The main difference between T2 and T3 is that T2 can be used with continuous laser (so no 'TTResult_SyncRate' that could make sense. But as you defined timestamps_unit = 1 / tags['TTResult_SyncRate']['value'] it gave the result you've seen from the file. So in T2 either timestamps or nanotimes don't make sense, there's only a "macrotime" from the start of the experiment. I chose to put it in timestamps but it should be maybe better to put it in nanotimes...

As you are much more used to all this branch/merge stuff, it would be nice if you could tidy all this... cheers

tritemio commented 5 years ago

Putting the macrotimes in 'timestamps' is correct, nanotime is for TCSPC data. We should fix the timestamp unit though. What is the relevant field/tag to use for T2 files?

seb5g commented 5 years ago

that's the nanotime units: ['MeasDesc_Resolution']

tritemio commented 5 years ago

nanotimes are meaningless for T2 files and the value in 'MeasDesc_Resolution' is not the timestamp resolution!

I dig it out by myself: the timestamps_unit is in the tag 'MeasDesc_GlobalResolution'.

I rebased your commit and merged it into master, adding unit tests and continuous integration. I also refactored the code removing the creation of the nanotimes array for T2 files which only wastes RAM.