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

Rework explain feature #971

Closed
3 tasks done
jmcook1186 opened this issue Aug 19, 2024 · 1 comment · Fixed by #996
Closed
3 tasks done

Rework explain feature #971

jmcook1186 opened this issue Aug 19, 2024 · 1 comment · Fixed by #996
Assignees

Comments

@jmcook1186
Copy link
Contributor

jmcook1186 commented Aug 19, 2024

What
Update behaviour of explainer feature. No we have it working and have been experimenting with it, we're not particularly happy with how we designed it, so we'll try making some behavioural changes to the feature.

Why
The block of information returned by explainer isn't that helpful for end users. The problem it is supposed to solve is that it is difficult to track what parameters are in a manifest. Today the explain block isn't much easier to read than the initialize block. We really need to rethink and make sure that a user can use the explain feature to understand what a parameter is and what it does.

Context

We should refocus on parameters, not plugins. We can understand plugins from their docs and the initialize information.

Instead of returning:

plugin-name:
  inputs:
    param-1:
      description:
      unit: 
      aggregation-method:

we should instead return:

param-name
  plugins: //what plugins use this param?
  unit: 
  description:
  aggregation-method:

Seting the parameter metadata can still be the same as it is today, but the logic and the way the data is displayed in explain needs to change.

Let's say two plugins both use cpu/energy. Both plugins have default metadata in their source code, and maybe it's overwritten in the manifest file - doesn't matter either way, the point is that metadata is available for both. It also doesn't matter whether the parameter is an input or an output in the plugins, we only care that it is used in some way. As long as the units and aggregation method are the same for each instance of the parameter, we can proceed, and should return the following in the explain block:

cpu/energy
  plugins: [plugin-1, plugin-2]
  unit: kWh
  description: energy consumed by the CPU in kWh
  aggregation-method: sum

However, if we see that the two instances of the parameter have different units or aggregation methods, we should just error out.

ERROR: your manifest uses two instances of cpu/energy with different units. Please check that you are using consistent units for cpu/energy throughout your manifest.

Prerequisites/resources

SoW (scope of work)

  • explain feature behaviour updated
  • documentation updated
  • test cases added

Acceptance criteria

Given (Setup): Describes the initial state of the system or the preconditions for a change.

When (Action): Describes the specific action or behavior that is being tested/changed.

Then (Assertion): Defines the expected outcome or behavior of the system after the action in the "When" step is performed.

@jmcook1186 jmcook1186 self-assigned this Aug 19, 2024
@jmcook1186 jmcook1186 added this to IF Aug 19, 2024
@zanete zanete added this to the Inputs and Outputs milestone Aug 21, 2024
@zanete zanete moved this from In Design to Ready in IF Aug 27, 2024
@zanete zanete moved this from Ready to In Progress in IF Aug 28, 2024
@zanete
Copy link

zanete commented Aug 28, 2024

only need to add new unit tests and update docs, PR expected today.

@manushak manushak moved this from In Progress to Pending Review in IF Aug 28, 2024
@manushak manushak linked a pull request Aug 28, 2024 that will close this issue
9 tasks
@zanete zanete closed this as completed Aug 29, 2024
@github-project-automation github-project-automation bot moved this from Pending Review to Done in IF Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants