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

feat: add corpus points to umap #917

Merged
merged 3 commits into from
Jul 17, 2023
Merged

feat: add corpus points to umap #917

merged 3 commits into from
Jul 17, 2023

Conversation

RogerHYang
Copy link
Contributor

resolves #894
resolves #899
resolves #900
resolves #901

Screen.Recording.2023-07-12.at.8.49.12.AM.mov

app/schema.graphql Show resolved Hide resolved
app/schema.graphql Show resolved Hide resolved
app/schema.graphql Show resolved Hide resolved
app/src/App.tsx Show resolved Hide resolved
src/phoenix/core/model_schema.py Show resolved Hide resolved
src/phoenix/core/model_schema.py Outdated Show resolved Hide resolved
src/phoenix/datasets/dataset.py Show resolved Hide resolved
src/phoenix/pointcloud/pointcloud.py Outdated Show resolved Hide resolved
@@ -15,3 +15,4 @@ class Context:
response: Optional[Response]
model: Model
export_path: Path
corpus: Optional[Model] = None
Copy link
Contributor

Choose a reason for hiding this comment

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

I get your point that this is the shortest path to victory - but I do think it does muddy things, particularly from a domain modeling perspective. Can we file a ticket to properly delineate these types into the behavior we expect them to exhibit? I get that they roughly map to tables of sorts, but there's also bespoke behavior that is definitely a non-semantic ontology here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok sounds good. to recap: Model is just some icing on a cake that's a table (in the sense of relational algebra), the trouble with corpus is that it has the cake but only half the icing, so it's much simpler to just keep all the icing on it, and maybe that's OK. Incidentally, what is a "Model" anyway if it mostly just walks and quacks like a table? Maybe what it needs is not surgery, but just a name change.

Copy link
Contributor

@mikeldking mikeldking left a comment

Choose a reason for hiding this comment

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

Approved - pending the outstanding comments

src/phoenix/__init__.py Outdated Show resolved Hide resolved
@RogerHYang
Copy link
Contributor Author

had to restrict hdbscan version for it to compile on python 3.8

      Error compiling Cython file:
      ------------------------------------------------------------
      ...
      import numpy as np
      cimport numpy as np

      from libc.float cimport DBL_MAX

      from dist_metrics cimport DistanceMetric
      ^
      ------------------------------------------------------------

      hdbscan/_hdbscan_linkage.pyx:12:0: 'dist_metrics.pxd' not found

@mikeldking
Copy link
Contributor

Failing build possibly due to scikit-learn-contrib/hdbscan#600

@mikeldking
Copy link
Contributor

merging since scikitlearn hdbscan issues are un-related

@mikeldking mikeldking merged commit 81428e9 into main Jul 17, 2023
@mikeldking mikeldking deleted the corpus-umap branch July 17, 2023 18:23
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
2 participants