Open
Conversation
yuvalrhino
requested changes
May 25, 2025
examples/nvflare/regression-quantile/QuantileRegression/model_parameters (1).joblib
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Can the inputs folder be called input or inputs and not input_quantile? That seems more natural and easier to use consistently across examples
| "name": "stdout", | ||
| "output_type": "stream", | ||
| "text": [ | ||
| "Requirement already satisfied: joblib in /Users/danieldavid/.pyenv/versions/3.11.6/lib/python3.11/site-packages (1.4.2)\n" |
Contributor
There was a problem hiding this comment.
Please clear all outputs from the notebook
Contributor
There was a problem hiding this comment.
Three comments:
- Can you please rename this file to something like
results_analysis.ipynbto be more consistent (and correctly spelled)? - What is the goal of this notebook? I see that it simply gets an input file that is checked into the directory and opens it - how is that performing any results analysis?
- If we're reorganizing this example - can we get rid of the
QuantileRegressionsubdirectory (under theregression-quantiledirectory)? Seems redundant.
|
|
||
| This folder contains examples for interacting with Rhino Health's Federated Computing Platform (FCP) using the Python SDK. | ||
|
|
||
| Each notebook demonstrates how to use the SDK to authenticate your user session, select a project, and perform a variety of federated analytics or compute tasks. |
Contributor
There was a problem hiding this comment.
Suggested change
| Each notebook demonstrates how to use the SDK to authenticate your user session, select a project, and perform a variety of federated analytics or compute tasks. | |
| Each notebook provides an example using the Rhino SDK to perform a different task e.g., importing datasets from external data sources, data preprocessing, federated analytics, or triggering model training. |
Comment on lines
+128
to
+129
| "FIRST_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the ID of the first test dataset\n", | ||
| "SECOND_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the ID of the second test dataset\n", |
Contributor
There was a problem hiding this comment.
Suggested change
| "FIRST_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the ID of the first test dataset\n", | |
| "SECOND_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the ID of the second test dataset\n", | |
| "FIRST_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the UID of the first test dataset\n", | |
| "SECOND_TEST_DATASET_UID = \"XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX\" # Replace this with the UID of the second test dataset\n", |
Contributor
There was a problem hiding this comment.
- Can you please put these files in an
inputssubdirectory? - Let's use consistent naming, e.g.
metrics_sample_data1.csv(consistent with the naming convention you used in the cox example).
| " ConnectionDetails,\n", | ||
| ")\n", | ||
| "from rhino_health.lib.metrics import Count, FilterType, Mean, StandardDeviation" | ||
| "from rhino_health.lib.constants import ApiEnvironment\n", |
Contributor
There was a problem hiding this comment.
Users shouldn't actually need to use ApiEnvironment
| "id": "3b107de9", | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "outputs": [ |
| "print(\"Logging In\")\n", | ||
| "my_username = \"my_email@example.com\" # Replace this with the email you use to log into Rhino Health\n", | ||
| "session = rh.login(username=my_username, password=getpass())\n", | ||
| "my_username = \"daniel.david@rhinohealth.com\" # Replace this with the email you use to log into Rhino Health\n", |
Contributor
There was a problem hiding this comment.
You shouldn't use your email in public examples - please keep this as my_email@example.org.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
verified and improved eda.ipynb, cox.ipynb, aggregate_quantile_example.ipynb. Improved markdown cells and README.md
cox.ipynb - verified, improved, and added 2 sample synthetic datasets. Organized it in a new directory.
eda.ipynb - verified, improved
aggregate_quantile_example.ipynb - verified, improved