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

KarhunenLoeveSVDAlgorithm seems to truncate the expansion from v1.19 #2139

Closed
mdelozzo opened this issue Sep 21, 2022 · 4 comments · Fixed by #2142
Closed

KarhunenLoeveSVDAlgorithm seems to truncate the expansion from v1.19 #2139

mdelozzo opened this issue Sep 21, 2022 · 4 comments · Fixed by #2142
Milestone

Comments

@mdelozzo
Copy link

mdelozzo commented Sep 21, 2022

Hello,

I'm upgrading openturns to 1.19 in gemseo and unfortunately a test fails due to a change in KarhunenLoeveSVDAlgorithm (maybe something in this PR).

The test relies on the following code (I removed the gemseo part and kept the openturns + numpy one). It considers a 100-length process sample over a 10-length 1D mesh. The number of eigenvalues was equal to 10 up to the version 1.18 and is equal to 3 with the version 1.19. As it uses KarhunenLoeveSVDAlgorithm without thresholding, the value 10 seems to me correct.

Does the v.1.19 truncate the expansion by default?

from numpy import array
from numpy import linspace
from numpy import pi
from numpy import sin
from numpy.random import rand
from openturns import Field
from openturns import KarhunenLoeveSVDAlgorithm
from openturns import Mesh
from openturns import ProcessSample
from openturns import Sample


def func(tau, theta):
    return sin(2 * pi * (tau - theta)) + 1


mesh_size = 10
mesh = [[t_i] for t_i in linspace(0, 1, mesh_size)]
data = array([func(array(mesh).flatten(), theta) for theta in rand(1000)])
mesh = Mesh(Sample(mesh))

datum = [[x_i] for x_i in data[0, :]]
samples = ProcessSample(1, Field(mesh, datum))
for datum in data[1:, :]:
    samples.add(Field(mesh, [[x_i] for x_i in datum]))

algo = KarhunenLoeveSVDAlgorithm(
    samples,
    [1] * mesh.getVerticesNumber(),
    0.0,
    True,
)
algo.run()

result = algo.getResult()
print(len(result.getEigenvalues()))
# 10 for version <= 1.18
# 3 for version == 1.19
@jschueller
Copy link
Member

jschueller commented Sep 22, 2022

on this example eigenvalues decrease very fast, and when we compute the cumulated sum for the criteria the sum stays the same from the 3rd value as we add a big value (~15) to a tiny one (9e-30):
eigenValues=[10.0389,2.68,2.25552,9.01814e-30,4.53074e-30,9.3953e-31,2.10531e-31,3.53615e-32,3.18668e-32,2.42483e-32]

@mdelozzo
Copy link
Author

Yes, you're right. But I think it is up to the user to remove the 7 non-significant eigenvalues.

@regislebrun
Copy link
Member

It should be enough to replace '<' by '<=' here

@mdelozzo
Copy link
Author

Your proposed fix seems to be good. Thanks.

@jschueller jschueller added this to the 1.20 milestone Sep 26, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 26, 2022
@jschueller jschueller mentioned this issue Sep 26, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 26, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 26, 2022
@jschueller jschueller linked a pull request Sep 26, 2022 that will close this issue
jschueller added a commit to jschueller/openturns that referenced this issue Sep 28, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 28, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 28, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 29, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Sep 30, 2022
jschueller added a commit to jschueller/openturns that referenced this issue Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants