nilmtk / nilmtk

Non-Intrusive Load Monitoring Toolkit (nilmtk)
http://nilmtk.github.io
Apache License 2.0
823 stars 458 forks source link

sample rate vs. sample period #887

Open klemenjak opened 4 years ago

klemenjak commented 4 years ago

Hi,

it's me again. I discovered a "naming conflict" in the source code. Let me explain:

The class API contains a variable sample_period. I guess this variable holds the time between two samples in seconds? A little down the code you fetch params['sample_rate'] from the definition of the experiment. As I looked up in the Jupyter notebooks, sample_rate is supposed to hand over the sample period.

class API():
    """
    The API is designed for rapid experimentation with NILM Algorithms. 
    """

    def __init__(self,params):

        """
        Initializes the API with default parameters
        """
        self.power = {}
        self.sample_period = 1
        ......
       self.sample_period = params['sample_rate']
      ........

This might be confusing for many people, including myself, because sample period and sample rate are two different things. A period is measured in seconds, whereas a rate gives information on some quantity per second. In our case:

This may result in people understanding that in the following experiment, we have 60 samples per second:

experiment = {
    'power': {
        'mains': ['apparent', 'active'],
        'appliance': ['apparent', 'active']
    },
    'sample_rate': 60,
.....

I'm pretty sure that 60 stands for 60 seconds and not for 60 Hertz.

I would suggest replacing sample_period by sample_period or sample interval so that it's clear that users have to provide values in seconds.

Feel free to ignore this in case you disagree. There are many opinions on this matter.

Best, Christoph

nipunbatra commented 4 years ago

Good point @klemenjak

@Rithwikksvr @PMeira Let us use sample_interval consistently across NILMTK and NILMTK-contrib? Thoughts?

Rithwikksvr commented 4 years ago

Yes @nipunbatra we need to use sample_interval. Also, we need to specify if user wishes to downsample the data.

PMeira commented 4 years ago

(Moving to NILMTK)