Skip to content

Conversation

jgerh
Copy link
Contributor

@jgerh jgerh commented Apr 11, 2025

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this 

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
@jgerh jgerh requested a review from terrykong April 11, 2025 16:49
@jgerh jgerh self-assigned this Apr 11, 2025
README.md Outdated

NVIDIA NeMo-Reinforcer is licensed under the [Apache License 2.0](https://github.com/NVIDIA/NeMo?tab=Apache-2.0-1-ov-file#readme).

NeMo is licensed under the [NVIDIA AI PRODUCT AGREEMENT](https://www.nvidia.com/en-us/agreements/enterprise-software/product-specific-terms-for-ai-products/). By pulling and using the container, you accept the terms and conditions of this license.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this right now? we actually don't publish a container yet. everything can be pip installed and we expect users to build their own container from the dockerfile we provide

Copy link
Contributor

Choose a reason for hiding this comment

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

@snowmanwwg do we need this language since we don't have a container? Can we drop?

@terrykong terrykong changed the title Tech pubs updates to file docs: large tech edit Apr 11, 2025
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 11, 2025
jgerh added 3 commits April 11, 2025 13:57
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Copy link
Contributor

@SahilJain314 SahilJain314 left a comment

Choose a reason for hiding this comment

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

Request to call 'Hugging Face' 'HuggingFace' universally? 'Hugging Face' is definitely the 'correct company name', but the community almost universally knows it as 'HuggingFace'. Want to avoid making the docs sound like a marketing blogpost.

I've generally suggested changes for brevity.

README.md Outdated
## Quick start
**Important Notes:**

- It is generally recommended **not to explicitly activate the virtual environment** when using `uv`. Instead, use `uv run <command>` to execute scripts within the managed environment. This helps maintain consistency across different shells and sessions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflicts with L50

Copy link
Contributor Author

@jgerh jgerh Apr 15, 2025

Choose a reason for hiding this comment

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

Please provide correction

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the 'activate the environment'? This line is correct.


## Slurm

The following code provides instructions on how to use Slurm to run batched job submissions and run jobs interactively.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The following code provides instructions on how to use Slurm to run batched job submissions and run jobs interactively.

docs/cluster.md Outdated
:::

To run interactively, launch the same command as the [Batched Job Submission](#batched-job-submission) except omit the `COMMAND` line:
To run interactively, launch the same command as the [Batched Job Submission](#batched-job-submission), except omit the `COMMAND` line:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To run interactively, launch the same command as the [Batched Job Submission](#batched-job-submission), except omit the `COMMAND` line:
To run interactively, launch the same command as in [Batched Job Submission](#batched-job-submission), but omit the `COMMAND` line:

jgerh added 8 commits April 14, 2025 15:11
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
Signed-off-by: Jennifer Gerhold <163925524+jgerh@users.noreply.github.com>
@terrykong
Copy link
Contributor

The state of the repo has moved very quickly and most if not all files are in conflict. I'll help @jgerh split out this PR

@terrykong
Copy link
Contributor

@jgerh closing in favor or #303 where i worked thru the rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants