Feat/22 models issue 195 #216
Conversation
TST: test against the finite-difference rms_laplacian.
|
Can you explain to me what this does and how it is different from the current |
|
We discussed it here: It does nothing new concerning fourier_synthesis, except maybe giving a bit more flexibility for the generation. The main idea is to group utilities such as computing rms heights and fractional derivatives for a self=affine model PSD in one place. Since this involves a full parametriyation of the PSD, we thought about adding fourier_synthesis a method for convenience. This is not yet ready for merging. |
|
Okay. It would be good not to duplicate things that are in Why does this need to be a class and is not a bunch of functions? (I.e. what is the state of the object?) |
|
this class calls fourier_synthesis. It is not duplicate |
|
The purpose of the class is actually to easily plot a self=affine model PSD to compare it to experimental 1D PSD and fit the parameters with the eyes. Once this is done we can generate a representative synthetic surface with Fourier Synthesis. |
|
My thought was to provide some flexibility on how to parametrize the Self-affine PSD via init, for example whether to give C0, Cr or wavelengths vs. wavevectors. Note that this is currently absolutely not implemented yet. |
|
Do you think this is bad code style ? Should I rather implement that via wrappers or in each function separately ? |
|
The state of the object should be set at init and then immutable. |
|
Classes should only be used if absolutely necessary, i.e. if they actually abstract some data. This may be the case here, they abstract the PSD data. Note that we could then write functions that act on such as class, such as Persson's model. The class should ideally be able to represent discretized (i.e. the average of a DST) PSDs. (This could be a subclass. I.e. you define an interface and implement your analytic PSDs. At some point we add the discrete PSD.) |
|
You should think about the interface first and implement that as an abstract class. Then derive your analytic PSD class from it. |
|
Makes sense, and then we would implement how to do the integrals over these discretized PSDs there. |
|
Makes sense. The Abstract class would actually encompass any statistical description of a roughness. |
|
Exactly. The |
|
Do you think it is Ok to call ContactMechanics in our SurfaceTopography tests ? (it is practical for the half derivative computation) Note actually that the computation of the full-contact elastic energy for finite-thickness or graded materials should be part of ContactMechanics, where we define the Greeen's functions. |
|
Yes, I have no objections using |
|
Ok I will add it separately in the installation workflow, for the CI testing. Not in requirements. |
|
Yes, please don't put it in the requirements. |
|
Just wonder what should actually be a pipeline function and what should be a method ... |
…odicFFTElasticHalfSpace are moved to ContactMechanics
|
@pastewka I think I adressed all the comments I suggest that we merge this first and do a release. This will allow to merge my pull request in ContactMechanics, and allow you to turn the scanning probe simulation into a pipeline. I will implement the integration of the ACF in a new branch. |
|
@pastewka do you know why half the tests just get cancelled ? |
|
CI is out of my power. tests pass but other wrkflow just cancel arbitrarily. |
|
Can you give me a hint for the docstrings ? |
If you click on "files changes" you should see the remaining comments. |
You need to increase the timeout. Including your new tests, CI takes longer than 30 minutes. |
|
I adressed requested changes and I increased timeout |
No description provided.