Skip to content

Conversation

choijon5
Copy link
Contributor

@choijon5 choijon5 commented Oct 15, 2025

Check that all user provided setting values are of the correct type/values to ensure there's no unexpected behavior.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 15, 2025
@choijon5 choijon5 requested review from jansel, oulgen and yf225 October 15, 2025 20:43
@choijon5 choijon5 force-pushed the settings_validation branch from d9b521d to 8b048a6 Compare October 15, 2025 22:25
Copy link
Contributor

@jansel jansel left a comment

Choose a reason for hiding this comment

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

Test failures?

@choijon5 choijon5 force-pushed the settings_validation branch 2 times, most recently from c0bef41 to 29b9304 Compare October 16, 2025 05:36
@choijon5 choijon5 force-pushed the settings_validation branch 2 times, most recently from e5b50f6 to 388fa82 Compare October 16, 2025 05:59
@choijon5
Copy link
Contributor Author

choijon5 commented Oct 16, 2025

@jansel looks like you added a lot of checks here: 6a2a5a2#diff-a24a8c490d66acd0226f70af3e8b5a392c4ef8cb6975140e25e373793c951771

Do you still want this to be merged? I thought using post_init might be a bit cleaner, but yours has more checks.

@jansel
Copy link
Contributor

jansel commented Oct 16, 2025

__post_init__ isn't ideal since users can mutate settings after it runs. Maybe we should validate later when start a compile or autotuning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants