Corrected IETS #240
Corrected IETS #240
Conversation
…y from which we read in getPPforce() is invalid?
…s Grabare-collected in python, so it crase when I access it in C++ stiffnessMatrix(); Solved by making FF global in HighLevel.py:perform_relaxation(); keeping debug-prints for reference (will be removed later)
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
- Coverage 46.45% 46.41% -0.05%
==========================================
Files 35 35
Lines 5179 5184 +5
==========================================
Hits 2406 2406
- Misses 2773 2778 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@yakutovicha this codecov complains again that my modifications lowered the coverage, and fails the checks? Can we turn it of somehow? |
Please ignore the failing test at the moment. I will fix it before our next meeting. Here is the issue #241 |
|
I have no time/capacity to review things at the moment, so I removed myself from the reviewers. |
There was a problem hiding this comment.
Just comments for now, pleae have a look especially on the makefile. Thank you @ProkopHapala !
| @@ -175,6 +175,9 @@ def main(argv=None): | |||
| tip_positions_x, tip_positions_y, tip_positions_z, lvec_scan = common.prepareScanGrids() | |||
| r_tips = np.array(np.meshgrid(tip_positions_x, tip_positions_y, tip_positions_z)).transpose(3, 1, 2, 0).copy() | |||
| evals, evecs = core.stiffnessMatrix(r_tips.reshape((-1, 3)), pp_positions.reshape((-1, 3)), which=which, ddisp=common.params["ddisp"]) | |||
| print("vib eigenval 1 min..max : ", np.min(evals[:, 0]), np.max(evals[:, 0])) | |||
There was a problem hiding this comment.
May, I ask you, why is the printing necessary?
There was a problem hiding this comment.
It is not necessary, but I think it is useful to see some summary in the output imediately, before you start plotting images. In case you see some non-sense value (NaNs, all zero, super high, super low) then you know immedietely something went wrong. So yes, it is just for debugging, but I think it is debugging which user will often do. The question is computing cost. I think it does not cost too much, but maybe it would be better do it inside C++ loop (that would be even cheaper).
| for i in range(which): | ||
| evecs[i] = np.zeros((n, 3)) | ||
| print("py.core.stiffnessMatrix() 1 ") |
There was a problem hiding this comment.
This is just a check that the stiffness matrix was initialized?
| // symmetrize - to make sure that our symmetric matrix solver work properly | ||
| double tmp; | ||
| tmp = 0.5*(dynmat.xy + dynmat.yx); dynmat.xy = tmp; dynmat.yx = tmp; | ||
| tmp = 0.5*(dynmat.yz + dynmat.zy); dynmat.yz = tmp; dynmat.zy = tmp; | ||
| tmp = 0.5*(dynmat.zx + dynmat.xz); dynmat.zx = tmp; dynmat.xz = tmp; | ||
| // solve mat | ||
| Vec3d evals; dynmat.eigenvals( evals ); Vec3d temp; | ||
| double eval_check = evals.a * evals.b * evals.c; | ||
| //if( fabs(eval_check) < 1e-16 ){ }; | ||
| if( isnan(eval_check) ){ printf( "C++ stiffnessMatrix[%i] is NaN evals (%g,%g,%g) \n", i, evals.a, evals.b, evals.c ); exit(0); } |
There was a problem hiding this comment.
I guess that we used to have couple of NaNs in the past, but we have used floating average to get rid of it in Python.
There was a problem hiding this comment.
I think in principle we should not have NaNs (we offset from zero for that reason). So I think it is better to have this check, to spot if there are really some NaNs to be able to debug that case.
| #CPPFLAGS= -fPIC -std=c++11 -O2 -mtune=native | ||
| CPPFLAGS= -fPIC -std=c++11 -O2 -mtune=native -fopenmp | ||
| #CPPFLAGS= -fPIC -std=c++11 -Og -g |
There was a problem hiding this comment.
Beware, I think that the changes in the makefile are not neccessary.
There was a problem hiding this comment.
Actually I don't understand why you removed the debug options from the Makefile. I would keep them there, so that we can debug it easily in future.
|
Codecov is finally tamed 🎉 |
fixes #71
I found error in the IETS simulations which was found by @ondrejkrejci (as I mentioned previously here #71 (comment) )
Basically the whole problem was that the pointer to the Forcefield grid become invalid in C++ because Python garbage-collected correspoding numpy array FF which was local to
perform_relaxation()function.To solve this issue I simply made this array global by adding
global FFat the begining ofperform_relaxation()function, which prevents the grabage collectro from de-allocating.I know the memory-managament in ppafm (especially python/C++ sharing) is pretty nasty. These bugs are hard to find. If you have some better ideas how to do this sharing let me know.