Regularization in higher dimensions #354
Conversation
|
Thank you @YuanbinLiu ! Some tests are still failing. Can we handle anything beyond 4D? If not, should we add this limitation to the documentation? |
It's not limited to 4D; higher dimensions are also fesible. I'll find time to check for errors, but the tests passed before I pushed the PR. |
|
@YuanbinLiu Thanks for the explanation! Test results can be dependent on the operating system |
I have tried several times, all passed. I didn't found some bugs. Can anyone test it on a different system? |
| f"Point and points must have the same dimensionality. Got {pn.shape[0]} and {preg.shape[1]}." | ||
| ) | ||
| hull = ConvexHull(preg) | ||
| return np.all(np.dot(hull.equations[:, :-1], pn) + hull.equations[:, -1] <= 1e-12) |
There was a problem hiding this comment.
Is this floating point comparison maybe brittle?
There was a problem hiding this comment.
It shouldn't be caused by this. I tested it on different computers with new environments, and it worked fine.
| fraction_list = [[1.0]] + [[0.0]] + [[0.5]] * 8 | ||
| calc_hull_ND = calculate_hull_nd(label) | ||
|
|
||
| assert np.allclose(calc_hull_3D.equations, calc_hull_ND.equations, atol=1e-6) |
| get_e_dist_hull_ND = get_e_distance_to_hull_nd( | ||
| calc_hull_ND, atom, {3: -0.28649227, 17: -0.25638457}, "REF_energy" | ||
| ) | ||
| assert np.allclose(get_e_dist_hull_3D, get_e_dist_hull_ND, atol=1e-6) |
|
@YuanbinLiu could you check all numerical tolerances again and especially for the failing test? It is likely just the tolerance |
| @@ -245,7 +245,7 @@ def test_regularization_for_three_element_system(test_dir, memory_jobstore, clea | |||
| for at in atoms | |||
| ] | |||
|
|
|||
| assert all(d >= -1e-6 for d in des) | |||
| assert np.all(np.array(des) >= -1e6) | |||
There was a problem hiding this comment.
Oh yes, missed that -, thats why test passed seems, I just saw the both methods returns 1e6 if it fails
https://github.com/YuanbinLiu/autoplex_pub/blob/76c29c6c3787c1cc4cd30ec13c3f42969da6296a/src/autoplex/fitting/common/regularization.py#L653
https://github.com/YuanbinLiu/autoplex_pub/blob/76c29c6c3787c1cc4cd30ec13c3f42969da6296a/src/autoplex/fitting/common/regularization.py#L653
There was a problem hiding this comment.
If some configurations have a distance from the convex hull exceeding 1e6, they will be excluded from the training set afterwards.
Are the values supposed to be non zero, positive , negative ? Is there any specific range this values are suppose to be ? Also with this new function added supporting n dimensions, do we still need 3d method or it can be replaced with this new function now ? Can you comment on this @YuanbinLiu ? |
They should be non-negative (i.e., >= 0). Actually, we no longer need 3D, as this has already been handled in the new function. |
| @@ -699,6 +699,7 @@ def preprocess_data( | |||
| regularization: bool = False, | |||
| retain_existing_sigma: bool = False, | |||
| scheme: str = "linear-hull", | |||
| element_order: list = None, | |||
There was a problem hiding this comment.
| element_order: list = None, | |
| element_order: list | None = None, |
list = None would cause a data type mismatch for list
| norm = np.cross(n_d[2] - n_d[0], n_d[1] - n_d[0]) | ||
| plane_norm = norm / np.linalg.norm(norm) | ||
| else: | ||
| A = n_d[:-1] - n_d[0] |
There was a problem hiding this comment.
maybe clear variable names would be a bit better?
We have upgraded the functionality to support high-dimensional convex hull calculations with regularization. Previously, it was limited to 3D convex hulls, but we are now able to handle higher-dimensional cases, such as 4D convex hulls for three-element systems.
Test 1: Verify whether the new code can produce the same convex hull results as the old version for the 3D case.
Test 2: Evaluate whether the new code can handle high-dimensional (>3D) convex hulls.