Skip to content

feat: single file descriptor#1072

Open
akoumpa wants to merge 7 commits intomainfrom
akoumparouli/single_file_descriptor
Open

feat: single file descriptor#1072
akoumpa wants to merge 7 commits intomainfrom
akoumparouli/single_file_descriptor

Conversation

@akoumpa
Copy link
Copy Markdown
Contributor

@akoumpa akoumpa commented Jan 16, 2026

What does this PR do ?

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

Changelog

  • Add specific line by line info of high level changes in this PR.

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 add or update any necessary documentation?

If you haven't finished some of the above items you can still open "Draft" PR.

Additional Information

  • Related to # (issue)

adil-a and others added 6 commits January 15, 2026 17:33
Signed-off-by: adil-a <adil.asif2000@hotmail.com>
Signed-off-by: adil-a <adil.asif2000@hotmail.com>
…ther (original behavior)

Signed-off-by: adil-a <adil.asif2000@hotmail.com>
Signed-off-by: adil-a <adil.asif2000@hotmail.com>
Signed-off-by: Alexandros Koumparoulis <akoumparouli@nvidia.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jan 16, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@akoumpa
Copy link
Copy Markdown
Contributor Author

akoumpa commented Jan 16, 2026

/ok to test a1b93b7

@akoumpa akoumpa changed the title Akoumparouli/single file descriptor feat: single file descriptor Jan 16, 2026
@akoumpa akoumpa marked this pull request as ready for review January 19, 2026 18:53
_write_metadata(output_files_data)
# Step 3: Write actual tensor data from input files to output files
_write_data(input_files_data, output_files_data, num_threads)
if use_staging:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

staging work is sitll here. is this needed? if not let's remove the staging changes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will let @akoumpa confirm, but I don't think we want the staging changes here. The idea is to only have a single file descriptor that we pass around instead of mode="a" appending to files.

Signed-off-by: Alexandros Koumparoulis <akoumparouli@nvidia.com>
@akoumpa
Copy link
Copy Markdown
Contributor Author

akoumpa commented Jan 20, 2026

/ok to test efcdcb1

save_sharded: bool = False,
consolidated_output_path: Optional[str] = None,
num_threads_consolidation: Optional[int] = None,
staging_dir: Optional[str] = None,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the update @akoumpa. It looks like there are other references to the staging approach still (here and in other files this PR touches)

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.

3 participants