Make sure that supercell_matrices of single-atom-displaced and rattled supercells are the same per default #258

QuantumChemist · 2024-11-18T13:18:52Z

As discussed here #243 (comment)

Another todo:

adjust Phonon gap parameters to Si defaults

…from hyperparameter header

… (hyper)params and full for Database type if not defined otherwise

…mp_ids

QuantumChemist · 2024-11-19T16:19:42Z

src/autoplex/auto/phonons/flows.py

+            self.supercell_settings[mp_id][
+                "supercell_matrix"
+            ] = supercell_matrix_job.output


@JaGeo , I restructured the code in a way that the reduce_supercell_size_job is only called once and the matrix is passed like that. But I have left the reduce_supercell_size_job (which only will be triggered if no matrix is gieven) in the other functions as it was before, too, in case a user wants to use the single-atom displ. and rattled supercell constructors separately.
I have thought a lot about it and I don't see a reason why single-atom displ. and rattled supercells of the same wf should have different settings. Do you think I shall still add the possibility?

It might be that someone wants it. At the moment, I also do not see a clear use case. Let's, for now, keep it with this single job. Just make sure everything works with this functionality for now and we just keep in mind that we don't have it implemented yet :).

I can see if I can maybe put some "NotImplemented" warning. In principle the user can still at the moment pass different settings if they call the supercell constructors separately.

Added a simple user info instead

QuantumChemist · 2024-11-20T09:48:01Z

*Si like silicon in the commit 🙈

YuanbinLiu · 2024-11-20T15:49:03Z

src/autoplex/data/phonons/flows.py

@@ -393,7 +393,7 @@ def make(
            supercell_matrix_job = reduce_supercell_size_job(
                structure=structure,
                min_length=self.supercell_settings.get("min_length", 12),
-                max_length=self.supercell_settings.get("max_length", 25),
+                max_length=self.supercell_settings.get("max_length", 20),


How is the default max length of 20 determined? Is this a commonly used value in the filed? If a lattice constant of a unit cell already exceeds 20, what would happen? Would the scale factor be set to 1?

the length of 20 appears to cause the least problems for the run time and yields good supercells. If a lattice parameter of a unit cells exceeds this value, it won't be scaled and stay like it is. But for the phonon part, so large unit cells are not intended to be used (on a regular basis).

For some large systems like MOFs, such situations are common, but since we can customize them, this shouldn't be an issue.

YuanbinLiu · 2024-11-20T15:53:25Z

tests/auto/test_auto_flows.py

@@ -584,7 +584,7 @@ def test_complete_dft_vs_ml_benchmark_workflow_m3gnet(
    assert complete_workflow_m3gnet.jobs[4].name == "complete_benchmark_mp-22905"
    assert responses[complete_workflow_m3gnet.jobs[-1].output.uuid][1].output[0][0][
               "benchmark_phonon_rmse"] == pytest.approx(
-        5.2622804443539355, abs=1.0  # bad fit data
+        5.2622804443539355, abs=3.0  # bad fit data, fluctuates between 4 and 7
    )



Does this slight fluctuation result from the fitting, right?

yes, we would need more fit data, but then the unit tests would also take longer.

YuanbinLiu · 2024-11-20T15:58:38Z

src/autoplex/auto/phonons/flows.py

@@ -442,7 +442,6 @@ def make(
                                            dft_references=dft_references,
                                            supercell_settings=self.supercell_settings,
                                            displacement=displacement,
-                                            # TODO add a hyper parameter here for the benchmark
                                            atomwise_regularization_parameter=atomwise_reg_parameter,


For the docstrings of CompleteDFTvsMLBenchmarkWorkflow, should we unify the formatting since some start with lowercase letters while others start with uppercase letters?

We should indeed agree on a convention here. This is something that I can fix in the documentation pull-request.

Sounds good.

YuanbinLiu · 2024-11-20T16:47:35Z

src/autoplex/fitting/common/mlip-phonon-defaults.json

@@ -2,15 +2,16 @@
  "GAP": {
    "general": {
      "at_file": "train.extxyz",
-      "default_sigma": "{0.0001 0.05 0.05 0}",
+      "default_sigma": "{0.0001 0.1 0.05 0}",


For accurate phonon calculations, achieving high precision in force evaluations is crucial. While I understand that specific settings have been made for each structure, I believe a default force sigma of 0.05 would be more appropriate.

As before, from the Si JCP2020 paper settings.

I still recommend to use 0.05. 0.1 still seems too large. For most phonon calculations, when I don't use single-atom rattling, I prefer not to enable the automatic sigma setting. In this case, I can use the default value of 0.05 for fitting.

Ok, then I will change this back! :)

YuanbinLiu · 2024-11-20T16:50:38Z

src/autoplex/fitting/common/mlip-phonon-defaults.json

      "energy_parameter_name": "REF_energy",
      "force_parameter_name": "REF_forces",
      "virial_parameter_name": "REF_virial",
      "sparse_jitter": 1.0e-8,
      "do_copy_at_file": "F",
      "openmp_chunk_size": 10000,
      "gp_file": "gap_file.xml",
-      "two_body": true,
+      "e0_offset": 2.0,


Why do we force e0_offset to equal 2 instead of using the default value of 0? This setting is not common in GAP fitting.

Because I put the Si settings from the JCP2020 paper. But I can also set it to 0 if you think that this is better.

Yes, please set it to 0. The program will automatically calculate this value, or the user can customize it. A value of 2 might be a specific setting for Si.

YuanbinLiu · 2024-11-20T16:52:41Z

src/autoplex/fitting/common/mlip-phonon-defaults.json

-      "l_max": 10,
-      "n_max": 12,
+      "l_max": 12,
+      "n_max": 10,


I would increase n_max before increasing l_max. Or we can stick to n_max=l_max=10.

Here, I also use Si settings from JCP2020 paper. But I can doublecheck that I didn't mix it up.

The default values here, including those mentioned earlier, are intended to be applicable to most systems. Increasing l_max would greatly impact computational efficiency.

Ok, then I'll change it back too.

QuantumChemist and others added 12 commits November 14, 2024 18:00

increase M3GNET RMSE tolereance because of fluctuations

3022c3b

prettify GAP parameters representation and remove GAP related things …

a01c5a6

…from hyperparameter header

rearrange file entires to make it look better, added user-defined for…

f3d35a0

… (hyper)params and full for Database type if not defined otherwise

fix unit test

1b7e6c0

fix unit test

3754c8c

small format fix

d76dc84

fix unit test

4dc423e

removed a comment

c02bfc3

decrease default max_length for rattled supercells

1b95862

Merge branch 'autoatml:main' into main

8d0c3a8

reducing max_length default a bit

0337f46

Merge branch 'main' of github.com:QuantumChemist/autoplex

e51db69

QuantumChemist marked this pull request as draft November 18, 2024 14:33

This comment was marked as outdated.

Sign in to view

QuantumChemist added 10 commits November 19, 2024 13:14

added missing doctsring

eacd697

set max_length to 20

c89312c

set max_length to 20

a508a53

add fallback to single-atom-dsipl- supercells

b8ae70c

minor changes in file format

10ac1e5

supercell_matrix is only determined once by reduce_supercell_size_job

bc5ca78

filter out supercell_matrix key to prevent problems with mismacthing …

cb4c995

…mp_ids

filter out supercell_matrix mpid key as mpids are not used here

9f02c13

adjust job number and remove print

d60d9fc

adjust defaults

75f2668

QuantumChemist commented Nov 19, 2024

View reviewed changes

QuantumChemist self-assigned this Nov 20, 2024

QuantumChemist added the enhancement New feature or request label Nov 20, 2024

QuantumChemist added 3 commits November 20, 2024 10:31

fix unit tests

6da2b0f

fix unit tests

14a9eb5

putting the phonon GAP defaults to SI settings

776384d

QuantumChemist added 4 commits November 20, 2024 11:10

fix unit test

6b3ceb2

fix unit test

125eec1

fix unit test

a0f3d83

added user info about supercell settings

f35ef28

QuantumChemist marked this pull request as ready for review November 20, 2024 11:03

QuantumChemist requested a review from YuanbinLiu November 20, 2024 11:05

Merge branch 'autoatml:main' into main

bbae8f5

YuanbinLiu reviewed Nov 20, 2024

View reviewed changes

QuantumChemist mentioned this pull request Nov 20, 2024

Updated Documentation for Autoplex, Including RSS Features #226

Merged

4 tasks

QuantumChemist added 2 commits November 20, 2024 18:07

addressing Yb's comments

bc7dec9

adjust unit test

5b35adf

YuanbinLiu approved these changes Nov 20, 2024

View reviewed changes

fix unit test

a84458d

QuantumChemist merged commit cb1393f into autoatml:main Nov 20, 2024
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make sure that supercell_matrices of single-atom-displaced and rattled supercells are the same per default #258

Make sure that supercell_matrices of single-atom-displaced and rattled supercells are the same per default #258

QuantumChemist commented Nov 18, 2024 •

edited

Loading

This comment was marked as outdated.

QuantumChemist Nov 19, 2024

JaGeo Nov 20, 2024 •

edited

Loading

QuantumChemist Nov 20, 2024

QuantumChemist Nov 20, 2024

QuantumChemist commented Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024 •

edited

Loading

YuanbinLiu Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024 •

edited

Loading

QuantumChemist Nov 20, 2024

YuanbinLiu Nov 20, 2024

QuantumChemist Nov 20, 2024

Make sure that supercell_matrices of single-atom-displaced and rattled supercells are the same per default #258

Make sure that supercell_matrices of single-atom-displaced and rattled supercells are the same per default #258

Conversation

QuantumChemist commented Nov 18, 2024 • edited Loading

This comment was marked as outdated.

Choose a reason for hiding this comment

JaGeo Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuantumChemist commented Nov 20, 2024

Choose a reason for hiding this comment

QuantumChemist Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuantumChemist Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuantumChemist commented Nov 18, 2024 •

edited

Loading

JaGeo Nov 20, 2024 •

edited

Loading

QuantumChemist Nov 20, 2024 •

edited

Loading

QuantumChemist Nov 20, 2024 •

edited

Loading