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

Update torch_patch.py #3174

Merged
merged 7 commits into from
Jan 27, 2023
Merged

Update torch_patch.py #3174

merged 7 commits into from
Jan 27, 2023

Conversation

S163669
Copy link
Contributor

@S163669 S163669 commented Jan 18, 2023

Removed the PositiveDefinite() function for the one from PyTorch to be used instead, as the way of checking was less precise than the one from PyTorch resulting in errors for certain cases.

More details can be found here on the Pyro forum:
https://forum.pyro.ai/t/positive-definiteness-error-due-to-import-of-pyro/4887/5

Removed the PositiveDefinite() function for the one from PyTorch to be used instead, as is what less precise resulting in errors for certain cases.
fehiepsi
fehiepsi previously approved these changes Jan 18, 2023
Copy link
Member

@fehiepsi fehiepsi left a comment

Choose a reason for hiding this comment

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

Thanks @S163669!

@fehiepsi
Copy link
Member

@S163669 Probably the test failing is due to this line. Could you try to see if changing it to

dist.MultivariateNormal(mu, scale_tril=scale_tril)

fixes the issues?

Passed scale_tril to dist.MultivariateNormal as keyword argument instead of positional argument as it otherwise gets identified as being a covariance matrix.
@S163669
Copy link
Contributor Author

S163669 commented Jan 20, 2023

@fehiepsi It should be good now :)

martinjankowiak pushed a commit that referenced this pull request Jan 21, 2023
* Update GP tutorials

* Update the noise instead of lengthscale
@fehiepsi
Copy link
Member

@S163669 could you merge the dev branch? It includes some fixes to make tests pass for the changes here.

@S163669
Copy link
Contributor Author

S163669 commented Jan 22, 2023

@fehiepsi I just did :)

@fehiepsi
Copy link
Member

@S163669 Looks like that the additional symmetric check of the upstream PositiveDefinite constraint is making GP tutorial failing. Could you add torch.set_default_tensor_type(torch.DoubleTensor) at the end of the first cell of that tutorial?

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@S163669
Copy link
Contributor Author

S163669 commented Jan 23, 2023

@fehiepsi Yes, it's done.

@fehiepsi
Copy link
Member

Sorry, I didn't run the tutorial until the end to catch all errors. Could you update the cell 27 to

X = torch.from_numpy(
    df[df.columns[2:4]].values.astype("float64"),
)

We need to use float64 numpy array after setting DoubleTensor. Thanks!

@S163669
Copy link
Contributor Author

S163669 commented Jan 27, 2023

No worries @fehiepsi . It is changed now!

@fehiepsi
Copy link
Member

Thanks, @S163669!

@fehiepsi fehiepsi merged commit 9a858da into pyro-ppl:dev Jan 27, 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.

None yet

2 participants