Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic - Plugin Parameter Metadata #761

Closed
8 tasks done
Tracked by #759
zanete opened this issue Jun 3, 2024 · 2 comments
Closed
8 tasks done
Tracked by #759

Epic - Plugin Parameter Metadata #761

zanete opened this issue Jun 3, 2024 · 2 comments
Assignees
Labels
EPIC Used to denote an issue that represents a whole epic. Core team only

Comments

@zanete
Copy link

zanete commented Jun 3, 2024

Background

As we have developed IF we have explored many methods for standardizing the units and associated metadata for plugins, bu twe still haven't settled on a good general purpose solution.

It is important to have some way to verify plugin metadata. The metadata includes at least the list of parameters and return values and their units. The units are critical because two plugins that both return carbon might have units of lbs C / kwH, gCO2eq, kg C etc etc. A plugin later in the pipeline that does some additional transformation on that carbon value needs to know it is getting the value in the expected unit. A plugin expecting gCo2 that receives kgCO2 will return a value with a 1000x error.

Reading the plugin documentation is one simple solution, but there's no guarantee that a plugin's documentation and code develop at the same pace and it could be difficult to determine what metadata was valid for plugin run in the past if the metadata information is only available in documentation that might subsequently have been updated or removed. For verifiability, auditability and re-executability we need an in-code solution.

Our initial solution was to include a list of parameters that can be used by plugins in a file, params.ts that comes bundled with IF. Plugin builders can add to the list by appending to it at runtime or providing a totally new parameter file that overrides our default one. The rationale is that everyone is clear about the units for specific named parameters, i.e. if you want to name a parameter carbon it has to have the units we define in params.ts (unless you provide a new params file, but this is deliberately an advanced feature).

However, this has turned out to be a fairly poor solution as it adds too much friction for plugin developers and it constrains people's ability to build freely on top of IF and build complex pipelines. It also forced us as the IF core team to impose certain standards that turned out to be very difficult to reason about while still keeping IF as general purpose as possible. For example, for the carbon intensity of electricity, do we choose grid/carbon-intensity? Plugins that do things with the carbon intensity of electricity might not need to just work with grid/carbon-intensity, they might also want to work with other names like electric/carbon-intensity, or perhaps a cloud has its own computed carbon intensity factoring in the on-site generation, which they might want to name cloud/carbon-intensity. There is no way to decide on the one name for every “thing” we want to compute.

This means we need a more flexible solution that balances the need for auditable metadata with the ability to build freely and expressively on IF with the minimum of developer friction.

Proposed solution

There are several parts to the proposed solution. First is to remove the params.ts file and the IF logic that checks the params file for parameter metadata. Instead, we can make use of the metadata field we already expose in our plugin interface and move the metadata definitions into the plugins themselves rather than an external file.

export const MyPlugin = (globalConfig: Config): Plugin => { 
  const metadata = { 
    kind: 'execute', 
    outputs: [ 
      energy: { 
        description: 'amount of energy utilised by the component', 
        unit: 'kWh'
        }, 
      ] 
    }; 
    
    . . . 
    
    return execute, metadata; }

The params file is also used to grab the aggregation method that the aggregate feature should use to aggregate the values for a given parameter across time or across components. This can be moved into the aggregate feature config instead and completely removed from the plugin metadata.
This means you only have to provide the information when it is actually needed. e.g.

aggregation: metrics: 
  - 'cpu/utilization' 
  - method: sum 
  - type: 'both’ 

Once this is done, we can develop an explainer feature that collates the metadata from all the plugins in a given pipeline and output it as a node in the manifest. This means users can always see what units were generated by each plugin in an execution pipeline and check that the units fed from one pipeline to another are consistent. It also opens the door to a future static analysis type feature that auto-audits the unit propagation through a pipeline.

The explainer block should look similar to:

explain: 
  - name: energy-amd
    plugin: amd-chip-energy-computer 
    description: blah blah 
    unit: 'kWh' 
  - name: energy-intel 
    plugin: intel-chip-energy-computer 
    description: blah blah 
    unit: 'kWh'

❓ Q: An alternative to consider is whether it is actually better for explainer to enrich the existing initialize block for each plugin to create a leaner manifest file. I probably lean towards enriching the initialize block personally, just to keep all the plugin metadata and config in one place in the manifest. It might also be good to rename initialize to plugins or similar to remove any naming confusion.

Finally, parameter mapping should be implemented so that we can automatically add the return values of one plugin to the inputs array under the name required by another plugin.
For example, let's say one of our plugins returned cloud/vendor by default, and another plugin required the same information but expected to receive cloud-vendor-name. We could provide a mapping field in the config for the first plugin so that the cloud/vendor data is actually appended to the manifest data as cloud-vendor-name. There are already ways to do this, but the mapping reduces the amount of redundant data in the final manifest.
It could look as follows:

initialize:
  plugins:
    cloud-metadata
      method: CloudMetadata
      path: "@grnsft/if-plugins"
      mapping:
        cloud/vendor: cloud-name

Related discussion:
#772

❓ Q: will we REQUIRE metadata for each plugin - will this be a breaking change that will obselete existing plugins?

Tasks

@zanete
Copy link
Author

zanete commented Jun 27, 2024

During feature sizing, the estimated t-shirt size by the team for this feature was XL

@zanete zanete moved this from In Design to In Refinement in IF Jun 27, 2024
@zanete
Copy link
Author

zanete commented Jul 12, 2024

Starting implementation on this on the 9th of July

@zanete zanete moved this from In Refinement to In Progress in IF Aug 22, 2024
@zanete zanete closed this as completed Sep 30, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in IF Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPIC Used to denote an issue that represents a whole epic. Core team only
Projects
Status: Done
Development

No branches or pull requests

2 participants