Added Gauss-Legendre Quadrature Fourier features #64

ltiao · 2021-12-15T18:11:12Z

No description provided.

vdutor

Nice PR - the code structure makes a lot of sense. I believe two things are missing right now:

a README in the fourier_features directory outlining the different methods and in which scenarios they can be used. This can be very brief as long as it is practical. We could use tables or diagrams for this.
It looks like there are no tests for a few of the GaussianQuadrature features?

vdutor · 2022-02-07T10:29:59Z

gpflux/layers/basis_functions/fourier_features/quadrature/gaussian.py

            warnings.warn(
-                "Quadrature Fourier feature approximation of kernels "
-                "with small lengthscale lead to unexpected behaviors!"
+                "Fourier feature approximation of kernels with small "


Please add what you mean by "small".

vdutor · 2022-02-07T10:37:50Z

gpflux/layers/basis_functions/fourier_features/quadrature/gaussian.py

+class QuadratureFourierFeatures(GaussHermiteQuadratureFourierFeatures):
+    """
+    Alias for `GaussHermiteQuadratureFourierFeatures`.
+    """
+
+    pass


Suggested change

class QuadratureFourierFeatures(GaussHermiteQuadratureFourierFeatures):

"""

Alias for `GaussHermiteQuadratureFourierFeatures`.

"""

pass

QuadratureFourierFeatures = GaussHermiteQuadratureFourierFeatures

""" Alias for `GaussHermiteQuadratureFourierFeatures` """

Maybe explain why we have this alias in place? Is this the most common and recommended quadrature rule?

vdutor · 2022-02-07T11:44:22Z

gpflux/layers/basis_functions/fourier_features/base.py

        super(FourierFeaturesBase, self).__init__(**kwargs)
+        assert isinstance(
+            kernel, self.SUPPORTED_KERNELS
+        ), f"Only the following kernels are supported: {self.SUPPORTED_KERNELS}"


Minor: You could make this message more informative by specifying which kernel the user used.

vdutor · 2022-02-07T11:46:47Z

gpflux/layers/basis_functions/fourier_features/quadrature/base.py

+
+    def _compute_bases(self, inputs: TensorType) -> tf.Tensor:
+        """
+        Compute basis functions.


Minor: the word "compute" made me first think the function would return a functor with the basis functions.

Suggested change

Compute basis functions.

Evaluate the 2L^D number of basis functions at `inputs`.

vdutor · 2022-02-07T11:48:23Z

gpflux/layers/basis_functions/fourier_features/quadrature/base.py

+from gpflux.types import ShapeType
+
+
+class QuadratureFourierFeaturesBase(FourierFeaturesBase):


Please give a type and short description of self.factors as it is being used in _compute_constant.

Suggested change

class QuadratureFourierFeaturesBase(FourierFeaturesBase):

class QuadratureFourierFeaturesBase(FourierFeaturesBase):

factors: Type

"""Description"""

vdutor · 2022-02-07T11:58:28Z

gpflux/layers/basis_functions/fourier_features/quadrature/gaussian.py



-class QuadratureFourierFeatures(FourierFeaturesBase):
+class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):


Suggested change

class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):

class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):

""" Base class for `GaussHermiteQuadratureFourierFeatures` and `GaussLegendreQuadratureFourierFeatures`.

vdutor · 2022-02-07T12:00:19Z

gpflux/layers/basis_functions/fourier_features/quadrature/gaussian.py

-        super(QuadratureFourierFeatures, self).__init__(kernel, n_components, **kwargs)
+
+
+class GaussHermiteQuadratureFourierFeatures(GaussianQuadratureFourierFeatures):


Is it possible to add a reference to which maths this class implements?

vdutor · 2022-02-07T12:00:44Z

gpflux/layers/basis_functions/fourier_features/quadrature/gaussian.py

-    def _compute_bases(self, inputs: TensorType) -> tf.Tensor:
-        """
-        Compute basis functions.
+class GaussLegendreQuadratureFourierFeatures(GaussianQuadratureFourierFeatures):


Is it possible to add a reference to which maths this class implements?

vdutor · 2022-02-07T12:02:39Z

gpflux/layers/basis_functions/fourier_features/quadrature/newton_cotes.py

+from gpflux.types import ShapeType
+
+
+class SimpsonQuadratureFourierFeatures(QuadratureFourierFeaturesBase):


Is it possible to add a reference to which maths this class implements?

vdutor · 2022-02-07T12:04:21Z

gpflux/layers/basis_functions/fourier_features/random/quasi.py

+
+
+class QuasiRandomFourierFeatures(RandomFourierFeatures):
+


Suggested change

ltiao added 17 commits December 15, 2021 15:59

added gauss-legendre quadrature

a8483bc

basic implementation completed

6e10cd0

formatting and added matern degrees of freedom utility method

61992ee

fixed formatting and type annotations

8c745b0

fixed formatting and type annotations

55d989a

updated tests

5b47864

updated tests

bb09fe8

added minimal scipy requirements

52e4381

disable support for 3.6

3f05fd8

added missing imports

0dcf4c6

formatting

031dc9a

make sure to call parent initializer first for input argument checks

e465f35

fixed imports

58da313

fixed imports

0cf22ff

added new modules

abd375a

fixed class hierarchy

8d04e69

fixed formatting

4173ff0

vdutor reviewed Feb 7, 2022

View reviewed changes

Louis Tiao and others added 5 commits February 8, 2022 18:12

added re-weighted version

8d2e9a7

complete subclass implementation

39e432e

added reweighted version

383df2a

fixed small bug

e9cbc77

added importance sampling for quadrature

6613736

	Compute basis functions.
	Evaluate the 2L^D number of basis functions at `inputs`.

		from gpflux.types import ShapeType


		class QuadratureFourierFeaturesBase(FourierFeaturesBase):



		class QuadratureFourierFeatures(FourierFeaturesBase):
		class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):

	class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):
	class GaussianQuadratureFourierFeatures(QuadratureFourierFeaturesBase):
	""" Base class for `GaussHermiteQuadratureFourierFeatures` and `GaussLegendreQuadratureFourierFeatures`.

		super(QuadratureFourierFeatures, self).__init__(kernel, n_components, **kwargs)


		class GaussHermiteQuadratureFourierFeatures(GaussianQuadratureFourierFeatures):

		from gpflux.types import ShapeType


		class SimpsonQuadratureFourierFeatures(QuadratureFourierFeaturesBase):

Added Gauss-Legendre Quadrature Fourier features #64

Are you sure you want to change the base?

Added Gauss-Legendre Quadrature Fourier features #64

Uh oh!

Conversation

ltiao commented Dec 15, 2021

Uh oh!

vdutor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants