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

Update compute tutorial #347

Merged
merged 2 commits into from
Jan 12, 2024
Merged

Update compute tutorial #347

merged 2 commits into from
Jan 12, 2024

Conversation

dongreenberg
Copy link
Contributor

No description provided.

Copy link
Contributor Author

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

:ref:`On-Demand Cluster` for instructions on first getting
cloud credentials set up.

3. **SageMaker Cluster (Alpha)**: Clusters that are created and managed
Copy link
Collaborator

Choose a reason for hiding this comment

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

just checking we want to remove sagemaker from this? this tutorial is still currently acting as the main compute tutorial until we split them up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for the intro I don't think it's necessary yet. Maybe we can link out to the cluster docs for people to explore the range of clusters / functionality on their own?

I actually think it was already removed from the notebook, it's just still in the rst.

.. parsed-literal::
:class: code-output

base servlet: Calling method install on module env_20230829_035957
Copy link
Collaborator

Choose a reason for hiding this comment

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

I cleared the output for a bunch of these longer log outputs. I think we also need to revisit which ones we set to be logger.info vs debug in terms of what processes we want to be logged during processes. and possibly in tutorials keep it at the info/error level for cleanliness if we want to show output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I thought I cleared all output before I pushed. Yes, makes sense. 

Copy link
Collaborator

Choose a reason for hiding this comment

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

yup you did clear them -- I generally like to leave them in to display the output of the code (for users that might just want to skim through without setting up yet), but removed the more cluttery setup/logs output

@carolineechen carolineechen merged commit 9703f55 into main Jan 12, 2024
9 checks passed
@dongreenberg dongreenberg deleted the update_compute_tutorial branch January 12, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants