Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce pydantic_v1 compatibility module for pydantic>=2.0.0 support #240

Merged

Conversation

ringohoffman
Copy link
Contributor

@ringohoffman ringohoffman commented Sep 27, 2023

Alternate to #213
Related: microsoft/DeepSpeed#4407

Inspired by LangChain's solution: https://github.com/langchain-ai/langchain/blob/64385c4eae4a4b8c27e73b09018f344f0b791f31/libs/langchain/langchain/pydantic_v1/__init__.py#L14-L17

mii can unpin pydantic, but instead of using the new pydantic v2 features, mii can use v2's alias for the v1 API in order to allow greater flexibility for mii users by allowing them to use either pydantic v1 or v2

Instead of upgrading deepspeed to pydantic v2, deepspeed can use pydantic v2's alias for the v1 API in order to unpin pydantic and allow greater flexibility for deepspeed users
@ringohoffman
Copy link
Contributor Author

ringohoffman commented Sep 27, 2023

@loadams LMK what you think of this solution. I like it because it avoids the pain of upgrading and immediately unblocks people who want to use deepspeed but have already upgraded to pydantic v2 (me), but also avoids impacting users that are unable to upgrade from pydantic v1

@ringohoffman
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Protopia AI"

@ringohoffman
Copy link
Contributor Author

Can these workflows be retried? I added the license header, although scripts/check-license.py doesn't work on any files for me locally, even ones that are already checked in. I'm on MacOS using bash. Not sure if that that is the problem.

@ringohoffman
Copy link
Contributor Author

Can these workflows be retried? I added the license header, although scripts/check-license.py doesn't work on any files for me locally, even ones that are already checked in. I'm on MacOS using bash. Not sure if that that is the problem.

@loadams 🙏 not sure if you were notified of my above comment so just tagging you

@ringohoffman
Copy link
Contributor Author

@loadams it looks like everything is passing. Can you let me know the result of your internal discussion as to whether this PR and microsoft/DeepSpeed#4407 will be acceptable?

Based on what we've discussed, I think the next steps would be to:

  1. Merge this PR
  2. Rerun workflows for Introduce pydantic_v1 compatibility module for pydantic>=2.0.0 support DeepSpeed#4407 and merge
  3. Revert .github/workflows/nv-torch-latest-v100.yaml Install dependencies URL

@ringohoffman
Copy link
Contributor Author

It seems like spacy is also employing a similar approach.

@loadams
Copy link
Contributor

loadams commented Sep 28, 2023

@loadams it looks like everything is passing. Can you let me know the result of your internal discussion as to whether this PR and microsoft/DeepSpeed#4407 will be acceptable?

Based on what we've discussed, I think the next steps would be to:

  1. Merge this PR
  2. Rerun workflows for Introduce pydantic_v1 compatibility module for pydantic>=2.0.0 support DeepSpeed#4407 and merge
  3. Revert .github/workflows/nv-torch-latest-v100.yaml Install dependencies URL

@ringohoffman - this seems reasonable, we will want feedback from @mrwyattii as well. but your next steps are correct.

@ringohoffman
Copy link
Contributor Author

ringohoffman commented Oct 4, 2023

@mrwyattii do you have a timeline for reviewing this? 🙏 This is blocking my team from using DeepSpeed due to our dependency on pydantic>=2.0.0.

@mrwyattii
Copy link
Contributor

@ringohoffman thank you for the contribution. I'm sorry that this is a blocker for your team. I will prioritize getting this reviewed (along with the accompanying DeepSpeed PR you shared).

@mrwyattii mrwyattii merged commit ab6b544 into microsoft:main Oct 9, 2023
3 checks passed
@ringohoffman ringohoffman deleted the pydantic-compatibility-module branch October 9, 2023 22: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.

3 participants