user-defined upsampling resolution

[mouhamadalmounayar]

Types of changes

• Enhancement (project structure, spelling, grammar, formatting)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

A description of the changes proposed in the Pull Request

Added the possibility to provide a custom upsampling resolution to time-sync in the model config.

• If no upsampling resolution is provided, it defaults to 1s
• If upsampling resolution does not meet criteria a Config Error is thrown.

[jmcook1186]

Hi @mouhamadalmounayar sorry for the delay reviewing this - we're just fixing another bug we found in time-sync before incorporating your PR.

[manushak]

Hi @mouhamadalmounayar I’ve reviewed the PR and it works well with simple time series like:

{timestamp: '2023-12-12T00:00:00.000Z', duration: 300}

However, it throws a defined error in more complex cases, such as this manifest

name: time-sync
description: successful path
tags:
initialize:
  output:
    - yaml
  plugins:
    'time-sync':
      method: TimeSync
      path: 'builtin'
      config:
        start-time: '2023-12-12T00:00:00.000Z'
        end-time: '2023-12-12T00:01:00.000Z'
        interval: 5
        allow-padding: true
        upsampling-resolution: 5
      parameter-metadata:
        outputs:
          energy-cpu:
            unit: KWH
            description: energy
            aggregation-method: sum
tree:
  children:
    child:
      pipeline:
        compute:
          - time-sync
      inputs:
        - timestamp: '2023-12-12T00:00:00.000Z'
          duration: 1
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:01.000Z'
          duration: 5
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:06.000Z'
          duration: 7
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:13.000Z'
          duration: 30
          energy-cpu: 0.001

Could you please take a look and fix it?

[mouhamadalmounayar]

Hi @manushak, since the upsampling resolution is set to 5s which is greater than a duration for an input here, 1s, it seems it's normal to fail no ?

[manushak]

@mouhamadalmounayar, you’re right. I’ve updated the manifest but still get the same error. The durations are now 5, 7, and 30.

name: time-sync
description: successful path
tags:
initialize:
  output:
    - yaml
  plugins:
    'time-sync':
      method: TimeSync
      path: 'builtin'
      config:
        start-time: '2023-12-12T00:00:01.000Z'
        end-time: '2023-12-12T00:01:00.000Z'
        interval: 5
        allow-padding: true
        upsampling-resolution: 5
      parameter-metadata:
        outputs:
          energy-cpu:
            unit: KWH
            description: energy
            aggregation-method: sum
tree:
  children:
    child:
      pipeline:
        compute:
          - time-sync
      inputs:
        - timestamp: '2023-12-12T00:00:01.000Z'
          duration: 5
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:06.000Z'
          duration: 7
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:13.000Z'
          duration: 30
          energy-cpu: 0.001

[mouhamadalmounayar]

@manushak This is also normal according to the constraints needed for the upsampling resolution. For the upsampling resolution not to cause issues for resampling it needs to be a divisor of all the durations, the interval and the paddings. 7 % 5 != 0. If you try a duration of 10 instead of 7 it should work. I discussed this with @jmcook1186 here #354

[narekhovhannisyan]

@mouhamadalmounayar Also we had changes on time sync after your PR which have fixed bugs. So please rebase your branch and resolve the conflicts.

[narekhovhannisyan]

@mouhamadalmounayar I'm unable to run the following manifest (seems all constraints are preserved in this case):

name: time-sync
description: successful path
tags:
initialize:
  output:
    - yaml
  plugins:
    'time-sync':
      method: TimeSync
      path: 'builtin'
      config:
        start-time: '2023-12-12T00:00:00.000Z'
        end-time: '2023-12-12T00:01:00.000Z'
        interval: 5
        allow-padding: true
        upsampling-resolution: 5
      parameter-metadata:
        outputs:
          energy-cpu:
            unit: KWH
            description: energy
            aggregation-method: 
              time: sum
              component: sum
tree:
  children:
    child:
      pipeline:
        compute:
          - time-sync
      inputs:
        - timestamp: '2023-12-12T00:00:00.000Z'
          duration: 5
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:06.000Z'
          duration: 10
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:16.000Z'
          duration: 30
          energy-cpu: 0.001

can you please check?

[mouhamadalmounayar]

@mouhamadalmounayar I'm unable to run the following manifest (seems all constraints are preserved in this case):

name: time-sync
description: successful path
tags:
initialize:
  output:
    - yaml
  plugins:
    'time-sync':
      method: TimeSync
      path: 'builtin'
      config:
        start-time: '2023-12-12T00:00:00.000Z'
        end-time: '2023-12-12T00:01:00.000Z'
        interval: 5
        allow-padding: true
        upsampling-resolution: 5
      parameter-metadata:
        outputs:
          energy-cpu:
            unit: KWH
            description: energy
            aggregation-method: 
              time: sum
              component: sum
tree:
  children:
    child:
      pipeline:
        compute:
          - time-sync
      inputs:
        - timestamp: '2023-12-12T00:00:00.000Z'
          duration: 5
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:06.000Z'
          duration: 10
          energy-cpu: 0.001
        - timestamp: '2023-12-12T00:00:16.000Z'
          duration: 30
          energy-cpu: 0.001

can you please check?

@narekhovhannisyan For this manifest, there is a gap of 1s between the first and second input which time-sync is trying to fill with a 5s input which is not possible. That is why an error is thrown.

[narekhovhannisyan]

@mouhamadalmounayar ok makes sense, in that case, can we have more granular error messages so they will state what's wrong ? Currently error message is the same for all cases: Upsampling resolution does not adhere to all constraints.

[mouhamadalmounayar]

@mouhamadalmounayar ok makes sense, in that case, can we have more granular error messages so they will state what's wrong ? Currently error message is the same for all cases: Upsampling resolution does not adhere to all constraints.

yes, on it.