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

logical error in isolated atom identification #276

Open
sandipde opened this issue Jan 5, 2024 · 1 comment
Open

logical error in isolated atom identification #276

sandipde opened this issue Jan 5, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@sandipde
Copy link
Contributor

sandipde commented Jan 5, 2024

At present any configuration with one atom is being ignored unless they are IsolatedAtom. https://github.com/ACEsuit/mace/blob/68f1bd462ef96aa953310fb07e3abceb3b004ff8/mace/data/utils.py#L221C13-L221C13
@davkovacs @ilyes319

I would propose just to use

        for idx, atoms in enumerate(atoms_list):
            isolated_atom_config =False

            if len(atoms) == 1:
                isolated_atom_config = atoms.info.get("config_type") == "IsolatedAtom"
                if isolated_atom_config:
                    if energy_key in atoms.info.keys():
                        atomic_energies_dict[
                            atoms.get_atomic_numbers()[0]
                        ] = atoms.info[energy_key]
                    else:
                        logging.warning(
                            f"Configuration '{idx}' is marked as 'IsolatedAtom' "
                            "but does not contain an energy."
                        )
            else:

        if not isolated_atom_config: atoms_without_iso_atoms.append(atoms)

        if len(atomic_energies_dict) > 0:
            logging.info("Using isolated atom energies from training file")

        atoms_list = atoms_without_iso_atoms
@sandipde
Copy link
Contributor Author

sandipde commented Jan 5, 2024

I believe this is better. tested. works fine

    if extract_atomic_energies:
        atoms_without_iso_atoms = []

        for idx, atoms in enumerate(atoms_list):
            isolated_atom_config = atoms.info.get("config_type") == "IsolatedAtom"
            #if len(atoms) == 1:
            if isolated_atom_config:
                if energy_key in atoms.info.keys():
                    atomic_energies_dict[
                        atoms.get_atomic_numbers()[0]
                    ] = atoms.info[energy_key]
                else:
                    logging.warning(
                        f"Configuration '{idx}' is marked as 'IsolatedAtom' "
                        "but does not contain an energy."
                    )
            else:
                atoms_without_iso_atoms.append(atoms)

        if len(atomic_energies_dict) > 0:
            logging.info("Using isolated atom energies from training file")

        atoms_list = atoms_without_iso_atoms

@ilyes319 ilyes319 added the bug Something isn't working label Jan 9, 2024
@ilyes319 ilyes319 self-assigned this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants