implemented 'torch.distributions.constraints.symmetric' checking if the tensor is symmetric at last 2 dimension.#68644
Conversation
…sor is symmetric in last 2 dimensions.
CI Flow Status⚛️ CI FlowRuleset - Version:
You can add a comment to the PR and tag @pytorchbot with the following commands: # ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun
# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slowFor more information, please take a look at the CI Flow Wiki. |
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 510f7b4 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
|
Looks great. Could you add as small test to test_constraints which tests this on a few matrices and verifies that this can be stacked with |
Sure, it is my pleasure. Thank you for giving me detailed advice. I will test it out soon! |
@neerajprad How about this design? inheritable_torch.distributions.constraints_for_matrices It is different from previous implementation styles in 'torch.distributions.constraints' and it requires more computation. But more logical way. |
|
This is the best way I think. I followed the design styles from 'test/distributions/test_distributions.py' which used 'unittest' package rather than 'pytest' package. |
|
Looks good to me! Thanks for contributing, I'll review your PR for the Wishart distribution after this. |
| t = torch.cuda.DoubleTensor if is_cuda else torch.DoubleTensor | ||
| return constraint_fn(*(t(x) if isinstance(x, list) else x for x in args)) | ||
|
|
||
| @pytest.mark.parametrize('constraint_fn, result, value', [(e[0], e[1], e[2]) for e in EXAMPLES]) |
There was a problem hiding this comment.
nit: why not just @pytest.mark.parametrize('constraint_fn, result, value', EXAMPLES)?
There was a problem hiding this comment.
You are right. Thanks for checking!
|
@neerajprad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@neerajprad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@neerajprad merged this pull request in bc3bdbc. |
|
CI errors are caused due to the tests about constraints.positive_definite: |
I didn't see any failures related to this. Could you point me to the test errors, are the failures deterministic? We should definitely skip the test (or add |
|
Breaks cuda 10.2 build, reverting, see e.g. https://github.com/pytorch/pytorch/runs/4318116960?check_suite_focus=true |
|
This pull request has been reverted by f14c16e. To re-land this change, follow these steps. |
…he tensor is symmetric at last 2 dimension. (#68644) Summary: Implemented submodule for #68050 Opened cleaned, final version of PR for #68240 Explanation: I am trying to contribute to PyTorch by implementing distributions for symmetric matrices like Wishart distribution and Inverse Wishart distribution. Although there is a LKJ distribution for the Cholesky decomposition of correlation matrices, it only represents equivalence to restricted form of Wishart distribution. [https://arxiv.org/abs/1809.04746](https://arxiv.org/abs/1809.04746) Thus, I started implementing Wishart distribution and Inverse Wishart distribution seperately. I added a short code about the 'torch.distributions.constraints.symmetric', which was not included in 'torch.distributions.constraints' previously. i.e., 'torch.distributions.constraints' contains module like 'positive_definite' constraints, but it just assumes symmetricity of the input matrix. [Link](https://github.com/pytorch/pytorch/blob/1adeeabdc0c8832420c091c5c668843768530d7f/torch/distributions/constraints.py#L466) So, I think it will be better if we have constraint checking symmetricity of the tensors in PyTorch. We may further utilize it like `constraints.stack([constraints.symmetric, constraints.positive_definite])` for the constraint of the covariance matrix in Multivariate Normal distribution, for example, to check if the random matrix is a symmetric positive definite matrix. cc fritzo neerajprad alicanb nikitaved Reviewed By: jbschlosser Differential Revision: D32599540 Pulled By: neerajprad fbshipit-source-id: 9227f7e9931834a548a88da69e4f2e9af7732cfe [ghstack-poisoned]
Summary: While implementing #68644, during the testing of 'torch.distributions.constraint.positive_definite', I found an error in the code: [location](https://github.com/pytorch/pytorch/blob/c7ecf1498d961415006c3710ac8d99166fe5d634/torch/distributions/constraints.py#L465-L468) ``` class _PositiveDefinite(Constraint): """ Constrain to positive-definite matrices. """ event_dim = 2 def check(self, value): # Assumes that the matrix or batch of matrices in value are symmetric # info == 0 means no error, that is, it's SPD return torch.linalg.cholesky_ex(value).info.eq(0).unsqueeze(0) ``` The error is caused when I check the positive definiteness of `torch.cuda.DoubleTensor([[2., 0], [2., 2]])` But it did not made a problem for `torch.DoubleTensor([[2., 0], [2., 2]])` You may easily reproduce the error by following code: ``` Python 3.9.7 (default, Sep 16 2021, 13:09:58) [GCC 7.5.0] :: Anaconda, Inc. on linux Type "help", "copyright", "credits" or "license" for more information. >>> import torch >>> const = torch.distributions.constraints.positive_definite >>> const.check(torch.cuda.DoubleTensor([[2., 0], [2., 2]])) tensor([False], device='cuda:0') >>> const.check(torch.DoubleTensor([[2., 0], [2., 2]])) tensor([True]) ``` The cause of error can be analyzed more if you give 'check_errors = True' as a additional argument for 'torch.linalg.cholesky_ex'. It seem that it is caused by the recent changes in 'torch.linalg'. And, I suggest to modify the '_PositiveDefinite' class by using 'torch.linalg.eig' function like the below: ``` class _PositiveDefinite(Constraint): """ Constrain to positive-definite matrices. """ event_dim = 2 def check(self, value): return (torch.linalg.eig(value)[0].real > 0).all(dim=-1) ``` By using above implementation, I get following result: ``` Python 3.9.7 (default, Sep 16 2021, 13:09:58) [GCC 7.5.0] :: Anaconda, Inc. on linux Type "help", "copyright", "credits" or "license" for more information. >>> import torch >>> const = torch.distributions.constraints.positive_definite >>> const.check(torch.cuda.DoubleTensor([[2., 0.], [2., 2.]])) tensor(True, device='cuda:0') >>> const.check(torch.DoubleTensor([[2., 0.], [2., 2.]])) tensor(True) ``` FYI, I do not know what algorithm is used in 'torch.linalg.eig' and 'torch.linalg.cholesky_ex'. As far as I know, they have same time complexity generally, O(n^3). It seems that in case you used special algorithms or finer parallelization, time complexity of Cholesky decomposition may be reduced to approximately O(n^2.5). If there is a reason 'torch.distributions.constraints.positive_definite' used 'torch.linalg.cholesky_ex' rather than 'torch.linalg.eig' previously, I hope to know. Pull Request resolved: #68720 Reviewed By: samdow Differential Revision: D32724391 Pulled By: neerajprad fbshipit-source-id: 32e2a04b2d5b5ddf57a3de50f995131d279ede49
|
This pull request has been reverted by f14c16e. To re-land this change, follow these steps. |
…he tensor is symmetric at last 2 dimension. (pytorch#68644) Summary: Implemented submodule for pytorch#68050 Opened cleaned, final version of PR for pytorch#68240 Explanation: I am trying to contribute to PyTorch by implementing distributions for symmetric matrices like Wishart distribution and Inverse Wishart distribution. Although there is a LKJ distribution for the Cholesky decomposition of correlation matrices, it only represents equivalence to restricted form of Wishart distribution. [https://arxiv.org/abs/1809.04746](https://arxiv.org/abs/1809.04746) Thus, I started implementing Wishart distribution and Inverse Wishart distribution seperately. I added a short code about the 'torch.distributions.constraints.symmetric', which was not included in 'torch.distributions.constraints' previously. i.e., 'torch.distributions.constraints' contains module like 'positive_definite' constraints, but it just assumes symmetricity of the input matrix. [Link](https://github.com/pytorch/pytorch/blob/7abc01e565acc86230389219613c349495b283a6/torch/distributions/constraints.py#L466) So, I think it will be better if we have constraint checking symmetricity of the tensors in PyTorch. We may further utilize it like `constraints.stack([constraints.symmetric, constraints.positive_definite])` for the constraint of the covariance matrix in Multivariate Normal distribution, for example, to check if the random matrix is a symmetric positive definite matrix. cc fritzo neerajprad alicanb nikitaved Pull Request resolved: pytorch#68644 Reviewed By: jbschlosser Differential Revision: D32599540 Pulled By: neerajprad fbshipit-source-id: 9227f7e9931834a548a88da69e4f2e9af7732cfe
Summary: While implementing pytorch#68644, during the testing of 'torch.distributions.constraint.positive_definite', I found an error in the code: [location](https://github.com/pytorch/pytorch/blob/9bca54f4b18a366f1503a325f7a7d429bd2e36c9/torch/distributions/constraints.py#L465-L468) ``` class _PositiveDefinite(Constraint): """ Constrain to positive-definite matrices. """ event_dim = 2 def check(self, value): # Assumes that the matrix or batch of matrices in value are symmetric # info == 0 means no error, that is, it's SPD return torch.linalg.cholesky_ex(value).info.eq(0).unsqueeze(0) ``` The error is caused when I check the positive definiteness of `torch.cuda.DoubleTensor([[2., 0], [2., 2]])` But it did not made a problem for `torch.DoubleTensor([[2., 0], [2., 2]])` You may easily reproduce the error by following code: ``` Python 3.9.7 (default, Sep 16 2021, 13:09:58) [GCC 7.5.0] :: Anaconda, Inc. on linux Type "help", "copyright", "credits" or "license" for more information. >>> import torch >>> const = torch.distributions.constraints.positive_definite >>> const.check(torch.cuda.DoubleTensor([[2., 0], [2., 2]])) tensor([False], device='cuda:0') >>> const.check(torch.DoubleTensor([[2., 0], [2., 2]])) tensor([True]) ``` The cause of error can be analyzed more if you give 'check_errors = True' as a additional argument for 'torch.linalg.cholesky_ex'. It seem that it is caused by the recent changes in 'torch.linalg'. And, I suggest to modify the '_PositiveDefinite' class by using 'torch.linalg.eig' function like the below: ``` class _PositiveDefinite(Constraint): """ Constrain to positive-definite matrices. """ event_dim = 2 def check(self, value): return (torch.linalg.eig(value)[0].real > 0).all(dim=-1) ``` By using above implementation, I get following result: ``` Python 3.9.7 (default, Sep 16 2021, 13:09:58) [GCC 7.5.0] :: Anaconda, Inc. on linux Type "help", "copyright", "credits" or "license" for more information. >>> import torch >>> const = torch.distributions.constraints.positive_definite >>> const.check(torch.cuda.DoubleTensor([[2., 0.], [2., 2.]])) tensor(True, device='cuda:0') >>> const.check(torch.DoubleTensor([[2., 0.], [2., 2.]])) tensor(True) ``` FYI, I do not know what algorithm is used in 'torch.linalg.eig' and 'torch.linalg.cholesky_ex'. As far as I know, they have same time complexity generally, O(n^3). It seems that in case you used special algorithms or finer parallelization, time complexity of Cholesky decomposition may be reduced to approximately O(n^2.5). If there is a reason 'torch.distributions.constraints.positive_definite' used 'torch.linalg.cholesky_ex' rather than 'torch.linalg.eig' previously, I hope to know. Pull Request resolved: pytorch#68720 Reviewed By: samdow Differential Revision: D32724391 Pulled By: neerajprad fbshipit-source-id: 32e2a04b2d5b5ddf57a3de50f995131d279ede49
Implemented submodule for #68050
Opened cleaned, final version of PR for #68240
Explanation:
I am trying to contribute to PyTorch by implementing distributions for symmetric matrices like Wishart distribution and Inverse Wishart distribution. Although there is a LKJ distribution for the Cholesky decomposition of correlation matrices, it only represents equivalence to restricted form of Wishart distribution. https://arxiv.org/abs/1809.04746 Thus, I started implementing Wishart distribution and Inverse Wishart distribution seperately.
I added a short code about the 'torch.distributions.constraints.symmetric', which was not included in 'torch.distributions.constraints' previously. i.e., 'torch.distributions.constraints' contains module like 'positive_definite' constraints, but it just assumes symmetricity of the input matrix. Link So, I think it will be better if we have constraint checking symmetricity of the tensors in PyTorch.
We may further utilize it like
constraints.stack([constraints.symmetric, constraints.positive_definite])for the constraint of the covariance matrix in Multivariate Normal distribution, for example, to check if the random matrix is a symmetric positive definite matrix.
cc @fritzo @neerajprad @alicanb @nikitaved