Skip to content

Conversation

@sanjaydasgupta
Copy link
Contributor

@sanjaydasgupta sanjaydasgupta commented Mar 12, 2024

This PR adds a boolean parameter save_ludwig_config_with_model_weights to the API's train() and experiment() methods that allows the caller to indicate that the user-provided ludwig-configuration should be added to the output directory (along with the model weights). When this parameter is set to True, a file named ludwig_config.json is added to the output directory. The ludwig config will thus be uploaded to HF along with the model weights whenever the API's upload_to_hf_hub() is called. The save_ludwig_config_with_model_weights parameter has a False default, so the feature will not affect pre-existing code or behavior. Enabling users to share or reproduce each other's results is important in many contexts, so publishing the Ludwig config along with a fine-tuned model's weights on HF will be a helpful.

This PR implements this issue.

This feature can be tested by the new integration test def test_ludwig_config_save(save_ludwig_config_with_model_weights, tmp_path) in test_model_training_options.py

@github-actions
Copy link

github-actions bot commented Mar 12, 2024

Unit Test Results

  6 files  ±       0    6 suites  ±0   13m 40s ⏱️ - 16m 25s
12 tests  - 2 981    7 ✔️  - 2 974    5 💤  - 7  0 ±0 
60 runs   - 2 969  30 ✔️  - 2 975  30 💤 +6  0 ±0 

Results for commit 7dcc2b4. ± Comparison against base commit 606c732.

♻️ This comment has been updated with latest results.

@sanjaydasgupta
Copy link
Contributor Author

Hi @alexsherstinsky , this submission is now ready for your review.

@alexsherstinsky
Copy link
Collaborator

Hi @alexsherstinsky , this submission is now ready for your review.

Hi @sanjaydasgupta -- in order to make a pull request ready for review, please click the button "Ready for review" -- then a number of people will be able to review it and offer their thoughts. Thank you very much!

@w4nderlust
Copy link
Collaborator

Sanjay can you help me understand the reson for this? The config is already in the output directory, not sure why you need to add it one more time.

@sanjaydasgupta
Copy link
Contributor Author

Hi @alexsherstinsky please take a look at my updates. Thank you.

@sanjaydasgupta
Copy link
Contributor Author

Hi @alexsherstinsky please take a look at my updates. Thank you.

I will fix the build errors my morning. Thanks!

Copy link
Collaborator

@alexsherstinsky alexsherstinsky left a comment

Choose a reason for hiding this comment

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

LGTM! @sanjaydasgupta -- very nicely done! Thank you so much for this contribution!

Once all the tests pass, please feel free to merge. Yay!

Copy link
Collaborator

@alexsherstinsky alexsherstinsky left a comment

Choose a reason for hiding this comment

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

Let us wait to make sure that all tests pass. Thank you.

sanjaydasgupta and others added 8 commits March 16, 2024 07:46
…ights' into save-ludwig-config-with-model-weights
…or presence of 'model_hyperparameters.json'

test_upload_utils.py: augmented '_build_fake_model_repo()' to add 'model_hyperparameters.json' at expected location
…ights' into save-ludwig-config-with-model-weights
…ights' into save-ludwig-config-with-model-weights
@alexsherstinsky alexsherstinsky merged commit c09d5dc into ludwig-ai:master Mar 17, 2024
@sanjaydasgupta sanjaydasgupta deleted the save-ludwig-config-with-model-weights branch March 17, 2024 14:27
@sanjaydasgupta sanjaydasgupta restored the save-ludwig-config-with-model-weights branch March 17, 2024 14:27
@sanjaydasgupta sanjaydasgupta deleted the save-ludwig-config-with-model-weights branch March 18, 2024 01:28
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.

4 participants