-
Notifications
You must be signed in to change notification settings - Fork 1
feat: allow specifying actuator in csv sample store #455
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
base: main
Are you sure you want to change the base?
Conversation
|
Checks Summary Last run: 2026-01-26T20:34:03.488Z Code Risk Analyzer vulnerability scan found 2 vulnerabilities:
Mend Unified Agent vulnerability scan found 3 vulnerabilities:
|
Co-authored-by: Alessandro Pomponio <10339005+AlessandroPomponio@users.noreply.github.com> Signed-off-by: Michael Johnston <66301584+michael-johnston@users.noreply.github.com>
- Moved all logic related experiments into ExperimentDescriptors - included constiutive properties - Two subclasses once for External (Replay) experiments one for Internal - This greatly simplifies CSVSampleStoreDescriptor - This also simplifies CSVSampleStore - Provide a warning and upgrade path for old formats.
Signed-off-by: Michael Johnston <66301584+michael-johnston@users.noreply.github.com>
SampleStoreReference parameters should be a dict but tests assumed it was a pydantic model
…aj_csv_source_extension
Code updated. Rereview required
|
@AlessandroPomponio What is the process to resolve the vulnerability scan issues? I can't see anything reported in pipeline. |
| The key here is that you **must** define which columns in the CSV are observed properties | ||
| and which are constitutive properties. | ||
| If you want to use the column names directly as observed/constitutive property names | ||
| you can pass a list to the relevant field. | ||
| If you want to define new observed/constitutive property names for each column you | ||
| can pass a dictionary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what AI suggest rewriting this to to make it clearer. I like this one better with the example next to the explanation. Feel free to edit.
NOTE: the code block renders incorrectly. Please click on EDIT for this comment to see the real suggestion
You must specify which CSV columns contain observed properties (measurements/results)
and which contain constitutive properties (input parameters/configurations).
**Two ways to map columns:**
1. **Use CSV column names as-is** - Pass a list:
```yaml
constitutivePropertyMap:
- cpu_value
- memory_gb
This uses cpu_value and memory_gb as both the column names AND property names.
- Rename columns - Pass a dictionary:
This reads from CSV columns
observedPropertyMap: wallClockRuntime: 'wall-clock runtime' throughput: 'requests_per_sec'
wall-clock runtimeandrequests_per_sec,
but names themwallClockRuntimeandthroughputin the experiment.
| idColumn: Column containing entity identifiers | ||
| generatorIdentifier: Optional identifier for the entity generator | ||
| experimentIdentifier: Experiment identifier | ||
| actuatorIdentifier: Actuator identifier (defaults to 'replay') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defaults to None in the signature
Co-authored-by: Alessandro Pomponio <10339005+AlessandroPomponio@users.noreply.github.com> Signed-off-by: Michael Johnston <66301584+michael-johnston@users.noreply.github.com>
Adds ability to specify an actuator when importing data from a CSV. This allows data obtained from elsewhere (maybe from ado) to appear as ado data (can be used as memoized data in a space with that experiment)
The actuator/experiment combination will be validated against existing actuators and if passed imported.
Notes: