Skip to content

fix for np.float32 serialization #589

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

Merged
merged 8 commits into from
Apr 7, 2023
Merged

fix for np.float32 serialization #589

merged 8 commits into from
Apr 7, 2023

Conversation

santiatpml
Copy link
Contributor

@santiatpml santiatpml commented Apr 7, 2023

Added tests for transforms and moved tune to a different set.

@santiatpml santiatpml requested a review from montanalow April 7, 2023 01:00
@santiatpml santiatpml requested a review from montanalow April 7, 2023 01:05
@@ -11,7 +11,7 @@ RUN cat /etc/apt/sources.list
RUN apt-get update && apt-get install -y postgresql-pgml-14

# Cache this, quicker
RUN pip3 install xgboost scikit-learn diptest torch lightgbm transformers datasets sentencepiece sentence_transformers sacremoses sacrebleu rouge
RUN pip3 install xgboost scikit-learn diptest torch lightgbm transformers datasets sentencepiece sentence_transformers sacremoses sacrebleu rouge protobuf
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not need protobuf, what requires that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finbert model needs protobuf.

SELECT pgml.transform(
    inputs => ARRAY[
        'Stocks rallied and the British pound gained.', 
        'Stocks making the biggest moves midday: Nvidia, Palantir and more'
    ],
    task => '{"task": "text-classification", 
              "model": "ProsusAI/finbert"
             }'::JSONB
) AS market_sentiment;

Copy link
Contributor

Choose a reason for hiding this comment

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

wow... that's... indicative of a whole new level... are they actually calling any rpcs, or are they just serializing in a round about way? I don't really care...

Copy link
Contributor Author

@santiatpml santiatpml Apr 7, 2023

Choose a reason for hiding this comment

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

I could not reproduce the error I saw before without protobuf on two other machines. I checked the model folder in cache and I don't see any reference to protobuf. Looks like we don't need it.

@santiatpml santiatpml merged commit e02eaff into master Apr 7, 2023
@santiatpml santiatpml deleted the santi-llm-apis branch April 7, 2023 15:00
SilasMarvin pushed a commit that referenced this pull request Oct 5, 2023
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.

2 participants