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

Graph-like API to traverse CMF metadata #127

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sergey-serebryakov
Copy link
Contributor

@sergey-serebryakov sergey-serebryakov commented Oct 3, 2023

Related Issues / Pull Requests

Description

This PR implements API to traverse CMF metadata in a graph-like manner.

  • The entry point is the MlmdGraph class that retrieves from metadata store pipelines, stages, executions and artifacts. It implements methods such as pipelines, stages, executions and artifacts that return lists of respective artifacts (Pipeline, Stage, Execution and Artifact).
  • Each node mentioned above has its own Python wrapper class that provides developer-friendly API to access node's parameters and traverse graph of machine learning concepts (e.g., get all stages of a pipeline or get all executions of a stage).
  • The graph API also provides the Properties class wrapper for MLMD's properties and custom_properties nodes' fields. This wrapper implements the Mapping API and automatically converts MLMD's values to Python values on the fly.

What changes are proposed in this pull request?

  • New feature (non-breaking change which adds functionality).
  • This change requires a documentation update.

Checklist:

  • My code follows the style guidelines of this project (PEP-8 with Google-style docstrings).
  • I have commented my code.
  • My code requires documentation updates, and I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

This commit adds doc strings and type annotation for CmfQuery class methods.
- References to `client` in the source code (e.g., CmfClient instead of CmfQuery) are removed.
- Several bugs related to checking input dict parameters are fixed (e.g., `d = d or {}` where it should be `if d is None: d= {}`). Now, the corrent object is returned when input dict is just empty.
- Missing doc strings and type annotations are added.
- Additional checks and log messages are added.
Fixing one possible bug related to accessing a column in a data frame when this data frame is empty.
Adding key mapper classes to help map source to target keys when copying dictionaries.
The API implements graph-like API to traverse CMF metadata in a graph-like manner. The entry point is the
`MetadataStore` class that retrieves from metadata store pipelines, stages, executions and artifacts. Users
can specify search query to specify what they want to return (the query is basically the value for the
`filter_query` parameter of the `ListOptions` class ML Metadata (MLMD) library).

Each node mentioned above (pipeline, stages, executions and artifacts) havs its own Python wrapper class that
provides developer-friendly API to access node's parameters and travers graph of machine learning concepts (e.g.,
get all stages of a pipeline or get all executions of a stage).

The graph API also provides the `Properties` wrapper for MLMD's properties and custom_propertied nodes' fields.
This wrapper implements the `Mapping` API and automatically converts MLMD's values to Python values on the fly.
- Adding unit tests (97% coverage).
- Renaming certain classes, redesigning implementation of several methods.
- Adding `Type` class that represents concepts such as ContextType, ExecutionType and ArtifactType in MLMD library.
@sergey-serebryakov sergey-serebryakov marked this pull request as ready for review October 16, 2023 01:55
@sergey-serebryakov sergey-serebryakov self-assigned this Oct 16, 2023
@sergey-serebryakov sergey-serebryakov added the enhancement New feature or request label Oct 16, 2023
@sergey-serebryakov
Copy link
Contributor Author

This PR includes multiple work-in-progress commits that are not relevant for the project history, so this PR needs to be "squashed and merged".

@sergey-serebryakov
Copy link
Contributor Author

sergey-serebryakov commented Oct 16, 2023

@annmary-roy @varkha-d-sharma This PR includes a unit test file in the test directory. You mentioned that there was a parallel activity focused on adding unit tests to the project. Please, let me know where you plan to have unit tests and I modify this PR accordingly.

PS - Unit tests in this PR require special MLMD database (/tmp/mlmd.sqlite) that Ann sent me. So, in general, tests will not run unless this exact file exists.

@sergey-serebryakov sergey-serebryakov mentioned this pull request Oct 16, 2023
7 tasks
"""Return stage of this execution."""
if self._stage is None:
self._stage = _graph_node(
Stage, self._db, one(self._db.store.get_contexts_by_execution(execution_id=self.id))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sergey-serebryakov Just a clarification, when calling "get_context_by_execution" will it return both the stage context and Pipeline context ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need "one" function here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants