Skip to content

Conversation

@holgerroth
Copy link
Collaborator

Fixes # .

Description

Use consistent file names and code structure.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

Signed-off-by: Holger Roth <[email protected]>
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 19, 2025

Greptile Summary

  • Standardized file naming across LLM tutorial modules (renamed *_job.py to job.py, src/hf_sft_peft_fl.py to client.py, removed src/ subdirectories)
  • Updated all references in notebooks and shell scripts to use new standardized filenames

Confidence Score: 2/5

  • This PR contains a critical import error that will cause runtime failures.
  • The restructuring is systematic but 08.5_llm_streaming/job.py has incorrect imports referencing the old src/ directory that no longer exists, which will cause immediate import failures when executed.
  • Pay close attention to examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.5_llm_streaming/job.py - the import paths must be fixed before merging.

Important Files Changed

Filename Overview
examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.5_llm_streaming/job.py Renamed from streaming_job.py to job.py but imports still reference old src/ directory structure causing import errors.

Sequence Diagram

sequenceDiagram
    participant User
    participant JobScript as "job.py"
    participant FedJob
    participant Server as "NVFlare Server"
    participant Client as "NVFlare Client"
    participant Trainer as "SFTTrainer"
    
    User->>JobScript: "Execute python job.py"
    JobScript->>FedJob: "Create FedJob with config"
    JobScript->>FedJob: "Add FedAvg controller"
    JobScript->>FedJob: "Add model persistor"
    JobScript->>FedJob: "Configure ScriptRunner for clients"
    JobScript->>FedJob: "Export and run simulator"
    
    FedJob->>Server: "Initialize server with controller"
    FedJob->>Client: "Initialize clients with client.py script"
    
    loop "For each federated round"
        Server->>Client: "Send global model"
        Client->>Trainer: "Load global model"
        Client->>Trainer: "Train on local data"
        Trainer-->>Client: "Return trained model"
        Client->>Server: "Send model updates"
        Server->>Server: "Aggregate updates (FedAvg)"
    end
    
    Server-->>User: "Training complete"
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

10 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

holgerroth and others added 4 commits November 19, 2025 17:47
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (5)

  1. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 29 (link)

    logic: train_script references old path but file moved to hf_sft_peft_fl.py (no src/ prefix)

  2. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 77 (link)

    logic: File path references src/ directory but file moved to hf_sft_model.py in root

  3. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 79 (link)

    logic: Module path references src. prefix but files moved to root directory

  4. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.5_llm_streaming/job.py, line 17-18 (link)

    logic: Imports reference src. module but files moved to root directory

  5. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 129 (link)

    syntax: 'Clinet IDs' is mis-spelled

12 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

@holgerroth
Copy link
Collaborator Author

/build

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (2)

  1. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 77-79 (link)

    logic: src/ directory was removed but still referenced here, causing import failure

  2. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.4_llm_quantization/job.py, line 135 (link)

    syntax: Help text says "default to 5" but default is 3

17 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

def main():
args = define_parser()
train_script = "src/hf_sft_peft_fl.py"
train_script = "job.py"
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: train_script should be hf_sft_peft_fl.py not job.py (self-reference causes circular execution)

Suggested change
train_script = "job.py"
train_script = "hf_sft_peft_fl.py"

@holgerroth holgerroth marked this pull request as draft November 19, 2025 23:08
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. examples/tutorials/self-paced-training/part-4_advanced_federated_learning/chapter-8_federated_LLM_training/08.5_llm_streaming/job.py, line 16-17 (link)

    logic: imports reference old src/ directory but files moved to current directory

19 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format

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.

1 participant