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

Audit existing plugins to determine what can be replaced with generics #640

Closed
2 of 3 tasks
Tracked by #629 ...
jmcook1186 opened this issue Apr 11, 2024 · 16 comments
Closed
2 of 3 tasks
Tracked by #629 ...
Assignees
Milestone

Comments

@jmcook1186
Copy link
Contributor

jmcook1186 commented Apr 11, 2024

Sub of: #656

What
Audit the existing set of plugins in if-plugins and if-unofficial-plugins and see what can be removed and replaced with combinations of generic builtins.

Why
As a user I want to simplify my IF experience, relying as little as possible on additional dependencies.

Context

Instead of having specific plugins for single use cases, it is better to support generics where they can provide the same outcome. This ticket covers an initial audit of our plugin repositories to determine which of our existing plugins can be replaced by a set of generic calculations instead. The outcomes should be a list of plugins that can be removed and a any generic builtins that should be added to maintain the same overall functionality.

Prerequisites/resources
None

Statement of Work
tbc

Acceptance criteria

  • document exists that identifies the agreed changes to each of our existing plugins across if if-plugins and if-unofficial-plugins
    - For each plugin we want to know whether it should be deprecated and deleted, moved to another repository or hand over to another org to maintain.
    - Any that are unclear should be discussed async or in a spike call if necessary

  • list exists that enumerates the new builtins that are required
    - Anticipate that some new builtins will be required to support existing plugins being deprecated, likely including subtract, exponent, interpolate and csv-lookup.

  • decision is made on what to do with tdp-finder and ccf

@manushak
Copy link
Contributor

Based on the Notion document and my audits, here is a conclusion on plugins

If-unofficial-plugins

Azure-importer - Leave in if-unofficial-plugins until moving to Microsoft
Boavizta - Leave in if-unofficial-plugins until moving to Boavizta
CCF - not sure what need to do with this plugin
Co2js - Leave in if-unofficial-plugins until moving to Green Web Foundation
Teads-aws and teads-curve - Replacing with Interpolation plugin in IF
Watt-time - I think we should leave it our responsibility to maintain it as it fundamental plugin

If-plugins

Coefficient - move to builtins
CSV Export - remove ?
Divide - move to builtins
E-mem and E-net - remove
Mock-observations - move to builtins in IF
Multiply - move to builtins
SCI - need a repo for SCI
SCI-M - move to SCI repo
SCI-O - delete
SCI-E - delete
Shell - move to builtins
Sum - move to builtins
Tdp-finder - remove it.
Cloud-metadata - need a repo for this plugin. Enhance the plugin to take only physical-processor name and return tdp

@jmcook1186 @jawache could you please verify?

@jawache
Copy link
Contributor

jawache commented Apr 17, 2024

Thanks @manushak , I agree with everything although we do need to treat the watttime plugin the same as everyone else to be fair.

For tdp-finder it was used in the hackathon by a team so it has been useful to someone and thinking about it it's very useful for people who are not on the cloud.

I think what we need to do is to make it work with cloud metadata so CM returns the processor family ONLY and then tdp-finder returns the TDP, so where right now we can use cloud-metadata only, in the future your would have to use both like so:

  • cloud-metadata
  • tdp-finder
  • teads-curve

Cloud metadata only works if you are running on one of the majority clouds, if you just have your own server then you need tdp-finder to get the TDP for your chip.

@manushak
Copy link
Contributor

Thanks @jawache. I think having a specification document for these plugins would be good. When I get to them, I'll create it and notify you to verify.

@jawache
Copy link
Contributor

jawache commented Apr 18, 2024

Thanks @manushak re: the spec document, is this in Google docs? I think we should write them up in a way that is useful in the future (markdown) I'd imagine they will be added to the readme, could you just add them here to this issue or an issue created for that task specifically? I find it's easier to keep the conversation in one place.

I also think for now we just need to focus on what the config is and inputs/outputs how the plugin works isn't as important we can make a lot of decisions with just knowing the config/inputs/outputs and we might be able to save some time. If we can't then we can dive deeper to write out a bigger spec.

@manushak
Copy link
Contributor

@jawache I think we should create a separate task for those plugins to avoid making this task heavier. It will also make testing easier, I guess.
It would be good to review config/inputs/outputs. We encountered a scenario where the user wanted the config to pass through the pipeline. I think we can pass configs through the pipeline without merging with the inputs/outputs

@jmcook1186
Copy link
Contributor Author

jmcook1186 commented Apr 18, 2024

Thanks for doing this @manushak !
Yes, there is a separate conversation to be had about dynamically setting node level config.

Looks like we need to spike on CCF, csv-export, watt-time, tdp-finder and cloud-metadata. The others seem decided.

@jmcook1186
Copy link
Contributor Author

jmcook1186 commented Apr 25, 2024

Also, if we are doing work to emphasise generic arithmetic plugins it makes sense to add subtract and exponent to the set. These could be good first issues, actually.

Discussion with @jawache also yielded the idea of a csv-lookup builtin that can lookup arbitrary values from any csv on the internet by passing in query terms as arguments.

@jmcook1186
Copy link
Contributor Author

jmcook1186 commented Apr 25, 2024

@jawache @manushak here's what I consider to be the current state of this discussion:

To deprecate and delete:

  • teads-curve
  • teads-AWS
  • Emem
  • Enet
  • sci-o
  • sci-e

Hand over:
(see also ##546 for progress of handover discussions)

  • azure-importer
  • boavizta
  • co2js
  • watt-time
  • cloud-metadata

Move to if/builtins

  • Coefficient
  • Divide
  • csv-export
  • mock-observations
  • multiply
  • shell
  • sum
  • tdp-finder
  • sci
  • sci-m (rename to sci-embodied

Not sure:

  • CCF

New plugins required

@jmcook1186 jmcook1186 self-assigned this Apr 25, 2024
@zanete
Copy link

zanete commented Apr 25, 2024

@zanete zanete self-assigned this Apr 25, 2024
@jawache
Copy link
Contributor

jawache commented Apr 26, 2024

@jmcook1186 agreed but also we need to perhaps have a place in the docs called common pipelines which shows how we might replicate say teads-curve with a set of generics, but also the equivalent for emem and enet etc...

also TDP-finder is too specific for builtins and I believe might be replaceable with lookup generic and a csv file, perhaps another to ask the RTC team to manage?

@jmcook1186
Copy link
Contributor Author

oh, that's a good point - our csv lookup builtin should be able to replace tdp-finder and maybe we can get someone else to maintain the file.

100% agree we should package up and distribute common pipelines - maybe this is something we can do via the registry website eventually?

@zanete
Copy link

zanete commented Apr 29, 2024

This will be complete:

  • decision is made on what to do with tdp-finder and ccf
  • an issue is created (one per) for moving set of plugins to builtins
  • an issue per New plugins required is created - can be parked until @manushak gets back on the project

This is to ensure that max plugins are already in their correct locations for the launch of the plugin registry

@zanete
Copy link

zanete commented May 8, 2024

@jawache the only thing standing in the way of closing this one is a decision on what to do with tdp-finder and ccf
Do you have any thoughts?
cc @jmcook1186

@jawache
Copy link
Contributor

jawache commented May 8, 2024

@zanete

ccf we should try to move to thoughtworks, so part of that issue is to offload plugins to the real owners (ccf is really a thoughtworks project)

tdp-finder should move to the internal RTC team (another GSF project).

Note both of these might take several months to complete.

@zanete
Copy link

zanete commented May 9, 2024

@jmcook1186 given above, I've added both to the #546, does that fit? and are you happy with the conclusion here so we can close? 🙏

@jmcook1186
Copy link
Contributor Author

Yes, that's fine @zanete - happy for this to be closed out.

@zanete zanete closed this as completed May 9, 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

No branches or pull requests

4 participants