ContactEngineering / SurfaceTopography

Read and analyze surface topographies
https://contactengineering.github.io/SurfaceTopography/
MIT License
17 stars 10 forks source link

Problems with version 0.92.0 or muFFT 0.15.0 #60

Closed mcrot closed 3 years ago

mcrot commented 3 years ago

I have some tests in Topobank which ran fine for SurfaceTopography version 0.91.5 and muFFT 0.11.1, but now with version 0.92.0 (SurfaceTopography) and 0.15.0 (muFFT) I have two failures.

First: Broadcast problem:

I've created a small example from my test:

import numpy as np
from SurfaceTopography import Topography

info = dict(unit='nm')

y = np.arange(10).reshape((1, -1))
x = np.arange(5).reshape((-1, 1))
arr = -2 * y + 0 * x  # only slope in y direction

topo = Topography(arr, (10, 5), info=info).detrend('center')

curv_x, curv_y = topo.derivative(n=2)
curv = curv_x[:, 1:-1] + curv_y[1:-1, :]

This results in

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-17-ccfe060953cb> in <module>
----> 1 curv = curv_x[:, 1:-1] + curv_y[1:-1, :]

ValueError: operands could not be broadcast together with shapes (3,6) (1,8) 

With the earlier version, it worked fine:

In [2]: curv
Out[2]: 
array([[0., 0., 0., 0., 0., 0., 0., 0.],
       [0., 0., 0., 0., 0., 0., 0., 0.],
       [0., 0., 0., 0., 0., 0., 0., 0.]])
mcrot commented 3 years ago

Second: Different slopes

This is more difficult to describe. Please look at the test function here.

The second last assert is

    np.testing.assert_almost_equal(series2['x'], exp_bins_y)

where I expect for the bins in y direction the values np.array([-4-1. / 3, -4, -4+1. / 3]) like for earlier versions, but it is now array([-4., -4., -4.]).

This is probably also related to how the derivative is calculated. Here

dh_dx, dh_dy = topography.derivative(n=1)

is used.

Are there changes in the way derivatives are calculated?

pastewka commented 3 years ago

Regarding the first problem

Derivative now excludes the boundary values for nonperiodic topographies. Just use curv = curv_x + curv_y.

Regarding the second problem

I don't think I understand what the test does. Your bins appear wrong now. But since you have a constant slope, there should only be a single bin. Could it be that before you had the derivative at the edges (over the nonperiodic boundary) in your data? But this should be a much larger value.

What are the actual derivative values that are calculated?

mcrot commented 3 years ago

Thank you. After looking at my tests, I think it does not make sense here to check the resulting distribution if we expect a delta peak here. So I think it's best to only check for the scalar values or to define a new example where the distribution values are exactly known.

The actual derivatives look as expected (here I'm using sizes (5,10) instead of (10,5) which as my original intention):

In [98]: unit = 'nm'
    ...: info = dict(unit=unit)
    ...: 
    ...: y = np.arange(10).reshape((1, -1))
    ...: x = np.arange(5).reshape((-1, 1))
    ...: 
    ...: arr = -2 * y + 0 * x  # only slope in y direction
    ...: 
    ...: t = Topography(arr, (5,10), info=info).detrend('center')

In [99]: t.positions_and_heights()
Out[99]: 
(array([[0., 0., 0., 0., 0., 0., 0., 0., 0., 0.],
        [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
        [2., 2., 2., 2., 2., 2., 2., 2., 2., 2.],
        [3., 3., 3., 3., 3., 3., 3., 3., 3., 3.],
        [4., 4., 4., 4., 4., 4., 4., 4., 4., 4.]]),
 array([[0., 1., 2., 3., 4., 5., 6., 7., 8., 9.],
        [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.],
        [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.],
        [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.],
        [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.]]),
 array([[ 9.,  7.,  5.,  3.,  1., -1., -3., -5., -7., -9.],
        [ 9.,  7.,  5.,  3.,  1., -1., -3., -5., -7., -9.],
        [ 9.,  7.,  5.,  3.,  1., -1., -3., -5., -7., -9.],
        [ 9.,  7.,  5.,  3.,  1., -1., -3., -5., -7., -9.],
        [ 9.,  7.,  5.,  3.,  1., -1., -3., -5., -7., -9.]]))

In [100]: t.derivative(n=1)
Out[100]: 
[array([[0., 0., 0., 0., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0., 0., 0., 0.]]),
 array([[-2., -2., -2., -2., -2., -2., -2., -2., -2.],
        [-2., -2., -2., -2., -2., -2., -2., -2., -2.],
        [-2., -2., -2., -2., -2., -2., -2., -2., -2.],
        [-2., -2., -2., -2., -2., -2., -2., -2., -2.]])]

In [101]: t.derivative(n=2)
Out[101]: 
[array([[0., 0., 0., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0., 0., 0.]]),
 array([[ 4.54747351e-15,  4.76802326e-16, -2.27373675e-15,
          1.04178889e-15, -5.68434189e-16, -1.04178889e-15,
         -1.13686838e-15, -4.76802326e-16],
        [ 4.54747351e-15,  4.76802326e-16, -2.27373675e-15,
          1.04178889e-15, -5.68434189e-16, -1.04178889e-15,
         -1.13686838e-15, -4.76802326e-16],
        [ 4.54747351e-15,  4.76802326e-16, -2.27373675e-15,
          1.04178889e-15, -5.68434189e-16, -1.04178889e-15,
         -1.13686838e-15, -4.76802326e-16]])]

The number of bins (3) was given by me through a parameter. Normally it would be automatically chosen to 8.

However, with the current code the resulting distribution for y and this example topography is meaningless:

{'name': 'Slope distribution (y direction)',
   'x': array([-2., -2., -2., -2., -2., -2., -2., -2.]),
   'y': array([2.50199979e+14,            nan, 2.50199979e+14, 2.50199979e+15,
          5.00399959e+14, 0.00000000e+00, 0.00000000e+00, 5.00399959e+14])},

We could think about to fix it in TopoBank, such that an artifical example as mine has a nice result.

However, I close this issue here, because it is not related to SurfaceTopography.