Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring of the MLIP modules and other non-urgent things #280

Merged
merged 107 commits into from
Dec 13, 2024

Conversation

QuantumChemist
Copy link
Collaborator

@QuantumChemist QuantumChemist commented Nov 29, 2024

I want to do a code refactoring and restructure some Makers and the MLIP modules, also check VASP Maker and custodian settings, plus improve the unit test setup (like using automate2 mock_vasp direct import, match unit tests to src structure).

This PR might take a while and will not change anything in the current code functionalities or features.

I am just collecting all issues that don't alter the functionality or features of the code under the umbrella of this PR. Tasks might have to be split up into several smaller PRs over time

current ToDos:

  • restructure MLIP input
  • refactor some Makers' make() parameters
  • update documentation with input changes
  • import mock_vasp via atomate2 monkeypatch
  • match unit tests to src structure
  • remove from __future__ import annotations from all files
  • rename rand_stuc labels and suffixes to rattled to distinguish it from the RSS part
  • better storing of the different xyz files
  • adjust the fit_kwargs handling in the docs
  • add to doc how to change MLIPPhononMaker settings (e.g. benchmark_kwargs={"relax_maker_kwargs": {"relax_cell": False, "relax_kwargs": "other_kwargs"}}

final ToDo:

  • check if documentation is up-to-date with all changes

@QuantumChemist QuantumChemist changed the title [WIP] Refactoring of the MLIP modules [WIP] Refactoring of the MLIP modules and other non-urgent things Nov 29, 2024
@QuantumChemist
Copy link
Collaborator Author

now I need to adjust the fit_kwargs handling in the docs

@QuantumChemist QuantumChemist marked this pull request as ready for review December 11, 2024 16:23
@QuantumChemist
Copy link
Collaborator Author

The PR is ready to be reviewed, just need to make the last adjustments to the docs

@QuantumChemist QuantumChemist self-assigned this Dec 11, 2024
@QuantumChemist QuantumChemist requested a review from JaGeo December 11, 2024 16:24
tests/conftest.py Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
src/autoplex/auto/phonons/flows.py Outdated Show resolved Hide resolved
@QuantumChemist
Copy link
Collaborator Author

QuantumChemist commented Dec 12, 2024

I would say that this PR is ready to be merged, if there is no other suggestion for improvement, @JaGeo 😃

@@ -160,24 +163,25 @@ In a similar way, the M3GNet fit hyperparameters can be passed using `make` as w

```python
complete_flow = CompleteDFTvsMLBenchmarkWorkflow(
ml_models=["M3GNet"], ...,
ml_models=["M3GNet"], ..., apply_data_preprocessing=True,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What preprocessing happens and would the whole fit work without it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data preprocessing sorts out data points with large forces. It's not implemented for anything else than GAP, but it will also divide the dataset into the different data types (phonon, rattled). Which means that most of the time, nothing happens during this step.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But does the workflow run without it?

Copy link
Collaborator Author

@QuantumChemist QuantumChemist Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fit won't automatically work without it because in this step the data is also divided up into train and test set, so one would need to provide these files.

@QuantumChemist QuantumChemist changed the title [WIP] Refactoring of the MLIP modules and other non-urgent things Refactoring of the MLIP modules and other non-urgent things Dec 13, 2024
@QuantumChemist QuantumChemist merged commit 1fa6e8a into autoatml:main Dec 13, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment