feat(pointcloud): Add HDBSCAN to point cloud #57

fjcasti1 · 2022-11-22T18:59:14Z

resolves #46

review-notebook-app · 2022-11-22T18:59:18Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

src/phoenix/__init__.py

src/phoenix/pcloud/pcloud.py

mikeldking · 2022-11-22T22:47:43Z

src/phoenix/pcloud/pcloud.py

+                    float(point.y),
+                    float(point.z),
+                ]
+                "clusterId": point.cluster_id


Need to propagate point metadata to the UI by populating a metaData field that is a dictionary.

I assume you mean each point will have a metadata field the same as position?

Following up on this with #63

mikeldking · 2022-11-22T22:48:50Z

src/phoenix/pcloud/projectors.py

+
+@dataclass(frozen=True)
+class UMAPProjector:
+    hyperparameters: Dict[str, Union[int, float, str]]


Make private - otherwise the programmer may think you can modify directly?

Thoughts on providing a reasonable default for the constructor?

IMO, if we want to pass defaults and/or run proper validation, we should create a separate class UMAPHyperparameter, and put defaults and validations in it. It would be much more transparent and easier. The downside is then we would have to implement all the options of the UMAP class, which I think is not ideal.

The reason I went with this approach is that we don't need default values. When we call UMAP we use
UMAP(**hyperparameters). That command expands the dictionary setting the values specified and leaving the rest as default. So, our defaults are UMAP's defaults.

About making it private -- It is already done, by using dataclasses with frozen=true

About making it private -- It is already done, by using dataclasses with frozen=true
Nice

As for the type - the problem from a DX perspective is that there now is no type hints for these values - making this projector harder to use as I don't have documentation on the parameters. pyright actually catches the boundary issues - not 100% sure how. Thoughts on just making a strict type for the parameters we think need tuning?

I am happy to create another class for the hyperparameters and have a private method that turns the class into a dictionary. To UMAP eyes it will be the same. To our users, it will be mostly the same and we can address new hyperparameters as they are requested. Or directly setting them all up.

Following up in #64

src/phoenix/pcloud/pcloud.py

mikeldking · 2022-11-22T23:05:23Z

src/phoenix/pcloud/pcloud.py

+        self.prediction_label = prediction_label
+        # self.prediction_score = prediction_score
+        self.actual_label = actual_label
+        # self.actual_score = actual_score
+        self.raw_text_data = raw_text_data
+        # self.link_to_data = link_to_data


rather than having flat attributes, it might be good to structure these constructs a bit in a class or dict.

particularly since feature data will need to be serialized - Unlike with the arize platform, all data needs to be serialized in bulk.

Agreed, I think it deserves a little conversation though. Don't know what direction to go particularly. I am tempted to call it metadata class since it will go in the metadata part of the json. But not too in love with that

Inference or Event attributes seems appropriate.

Following up in #65

src/phoenix/pcloud/pcloud.py

src/phoenix/pcloud/projectors.py

Co-authored-by: Mikyo King <mikyo@arize.com>

…eat/add-hdbscan * 'feat/add-hdbscan' of github.com:Arize-ai/phoenix: Update src/phoenix/pcloud/projectors.py Update src/phoenix/pcloud/pcloud.py

Co-authored-by: Mikyo King <mikyo@arize.com>

…eat/add-hdbscan * 'feat/add-hdbscan' of github.com:Arize-ai/phoenix: black more fixes fix types: Update src/phoenix/pcloud/projectors.py Update src/phoenix/pcloud/pcloud.py Delete _fit_transform method Fix json int bug Fix style Add HDBSCAN clustering Restore umap_demo deleted fix: python types (#55)

.github/workflows/pull-requests.yaml

mikeldking · 2022-11-28T16:35:17Z

src/phoenix/pointcloud/pointcloud.py

+    y: float
+
+    def get_coordinates(self):
+        return [float(self.x), float(self.y)]


A bit confused on the casting to float here - is it for type safety?

That is because the default coordinates spit from UMAP and HDBSCAN have been float32 and, if left like that, you get the error that is not JSON serializable. I am open to other solutions.

Co-authored-by: Mikyo King <mikyo@arize.com>

Restore umap_demo deleted

e091861

Add HDBSCAN clustering

4d930c0

fjcasti1 marked this pull request as ready for review November 22, 2022 22:42

Francisco Castillo added 2 commits November 22, 2022 14:44

Fix style

07554f9

Fix json int bug

79df449

mikeldking reviewed Nov 22, 2022

View reviewed changes

Francisco Castillo and others added 11 commits November 22, 2022 15:19

Update src/phoenix/pcloud/pcloud.py

35e720d

Co-authored-by: Mikyo King <mikyo@arize.com>

Update src/phoenix/pcloud/projectors.py

d93c205

Co-authored-by: Mikyo King <mikyo@arize.com>

Delete _fit_transform method

d89db7f

Merge branch 'feat/add-hdbscan' of github.com:Arize-ai/phoenix into f…

8cdb046

…eat/add-hdbscan * 'feat/add-hdbscan' of github.com:Arize-ai/phoenix: Update src/phoenix/pcloud/projectors.py Update src/phoenix/pcloud/pcloud.py

Restore umap_demo deleted

b16870a

Add HDBSCAN clustering

0cd2ad6

Fix style

c0e6c58

Fix json int bug

4e33c4c

Delete _fit_transform method

54ff201

Update src/phoenix/pcloud/pcloud.py

e29c344

Co-authored-by: Mikyo King <mikyo@arize.com>

Update src/phoenix/pcloud/projectors.py

0970ab3

Co-authored-by: Mikyo King <mikyo@arize.com>

mikeldking force-pushed the feat/add-hdbscan branch from 8cdb046 to 0970ab3 Compare November 23, 2022 03:51

mikeldking and others added 10 commits November 22, 2022 21:27

fix types:

bf8a7b2

more fixes

e11b628

black

8c3e660

Add .coverage to gitignore

b5d390c

Add tables dependency toml file

affc479

wip

907ed4d

Use pull_request_target, better for forked repos

d119f2f

Add PR lint action

30892da

Not use pull_request_target

912b028

fjcasti1 changed the title ~~feat: Add HDBSCAN to point cloud~~ Add HDBSCAN to point cloud Nov 25, 2022

Francisco Castillo added 5 commits November 25, 2022 02:36

Add edited trigger

fccf925

Ignore type hints from umap and hdbscan

a30c198

Rename module pcloud -> pointcloud

75804d5

Ignore E203 flake8

5606eef

HDBSCAN finished

8cecd21

fjcasti1 changed the title ~~Add HDBSCAN to point cloud~~ feat(pointcloud): Add HDBSCAN to point cloud Nov 25, 2022

Francisco Castillo added 4 commits November 25, 2022 15:43

wip

226d6b8

Define and use Coordinates classes

7570608

Parametrize HDBSCAN hyperparams

38d7e13

Rename to DriftPointCloud

3b7229c

mikeldking reviewed Nov 28, 2022

View reviewed changes

.github/workflows/pull-requests.yaml Outdated Show resolved Hide resolved

mikeldking reviewed Nov 28, 2022

View reviewed changes

mikeldking approved these changes Nov 28, 2022

View reviewed changes

This was referenced Nov 28, 2022

Propagate point metadata to the UI by populating a metaData field #63

Closed

Create UMAPHyperparameters object #64

Closed

Create InferenceAttributes class #65

Closed

Update .github/workflows/pull-requests.yaml

c16c0d9

Co-authored-by: Mikyo King <mikyo@arize.com>

fjcasti1 merged commit 9aa2a3f into main Nov 28, 2022

fjcasti1 deleted the feat/add-hdbscan branch November 28, 2022 17:43

fjcasti1 mentioned this pull request Nov 28, 2022

Add UMAP and HDBScan. Run UMAP and output as pandas df and JSON serialization #4

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pointcloud): Add HDBSCAN to point cloud #57

feat(pointcloud): Add HDBSCAN to point cloud #57

fjcasti1 commented Nov 22, 2022 •

edited by mikeldking

Loading

review-notebook-app bot commented Nov 22, 2022

mikeldking Nov 22, 2022

fjcasti1 Nov 28, 2022

mikeldking Nov 28, 2022

fjcasti1 Nov 28, 2022

mikeldking Nov 22, 2022

mikeldking Nov 22, 2022

fjcasti1 Nov 28, 2022 •

edited

Loading

fjcasti1 Nov 28, 2022

mikeldking Nov 28, 2022

fjcasti1 Nov 28, 2022

fjcasti1 Nov 28, 2022

mikeldking Nov 22, 2022

mikeldking Nov 22, 2022

fjcasti1 Nov 28, 2022

mikeldking Nov 28, 2022

fjcasti1 Nov 28, 2022

mikeldking Nov 28, 2022

fjcasti1 Nov 28, 2022

feat(pointcloud): Add HDBSCAN to point cloud #57

feat(pointcloud): Add HDBSCAN to point cloud #57

Conversation

fjcasti1 commented Nov 22, 2022 • edited by mikeldking Loading

review-notebook-app bot commented Nov 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjcasti1 Nov 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjcasti1 commented Nov 22, 2022 •

edited by mikeldking

Loading

fjcasti1 Nov 28, 2022 •

edited

Loading