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

Changed the uhf value when xtb calculates with lanthanides #60

Merged

Conversation

jonathan-schoeps
Copy link
Contributor

The xtb calculation when lanthanides are present in the molecule now calculates with uhf = 0 to avoid shifting the uhf assignment to the ligands. When the xtb run is finished, the original uhf is reassigned to the molecule. This is linked to #50

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
@jonathan-schoeps jonathan-schoeps added the bug Something isn't working label Oct 9, 2024
@jonathan-schoeps jonathan-schoeps self-assigned this Oct 9, 2024
@jonathan-schoeps jonathan-schoeps linked an issue Oct 9, 2024 that may be closed by this pull request
Copy link
Member

@marcelmbn marcelmbn left a comment

Choose a reason for hiding this comment

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

Have you checked these two aspects:

  • The number of unpaired electrons with which xtb is started. You could also have a look at the command-line call
  • Is the UHF value actually set back to the original value afterwards?

CHANGELOG.md Outdated Show resolved Hide resolved
src/mindlessgen/qm/xtb.py Outdated Show resolved Hide resolved
src/mindlessgen/qm/xtb.py Outdated Show resolved Hide resolved
@jonathan-schoeps
Copy link
Contributor Author

Have you checked these two aspects:

* The number of unpaired electrons with which `xtb` is started. You could also have a look at the command-line call

* Is the `UHF` value actually set back to the original value afterwards?
  1. The command line call has no --uhf flag. And within each xtb run the unpaired electrons are 0.
  2. The UHF value is correctly reassinged. The test was with this print statements:

def single_molecule_generator(
    config: ConfigManager,
    refine_engine: QMMethod,
    postprocess_engine: QMMethod | None,
    cycle: int,
    stop_event,
) -> Molecule | None:
    """
    Generate a single molecule.
    """
    if stop_event.is_set():
        return None  # Exit early if a molecule has already been found

    if config.general.verbosity == 0:
        # print the cycle in one line, not starting a new line
        print("✔", end="", flush=True)
    elif config.general.verbosity > 0:
        print(f"Cycle {cycle + 1}:")
    #   _____                           _
    #  / ____|                         | |
    # | |  __  ___ _ __   ___ _ __ __ _| |_ ___  _ __
    # | | |_ |/ _ \ '_ \ / _ \ '__/ _` | __/ _ \| '__|
    # | |__| |  __/ | | |  __/ | | (_| | || (_) | |
    #  \_____|\___|_| |_|\___|_|  \__,_|\__\___/|_|

    try:
        mol = generate_random_molecule(config.generate, config.general.verbosity)
    # RuntimeError is not caught here, as in this part, runtime errors are not expected to occur
    # and shall therefore be raised to the main function
    except (
        SystemExit
    ) as e:  # debug functionality: raise SystemExit to stop the whole execution
        if config.general.verbosity > 0:
            print(f"Generation aborted for cycle {cycle + 1}.")
            if config.general.verbosity > 1:
                print(e)
        stop_event.set()
        return None

    print("Which uhf goes in:", mol.uhf)

    try:
        #    ____        _   _           _
        #   / __ \      | | (_)         (_)
        #  | |  | |_ __ | |_ _ _ __ ___  _ _______
        #  | |  | | '_ \| __| | '_ ` _ \| |_  / _ \
        #  | |__| | |_) | |_| | | | | | | |/ /  __/
        #   \____/| .__/ \__|_|_| |_| |_|_/___\___|
        #         | |
        #         |_|
        optimized_molecule = iterative_optimization(
            mol=mol,
            engine=refine_engine,
            config_generate=config.generate,
            config_refine=config.refine,
            verbosity=config.general.verbosity,
        )

        print("Which uhf comes out:", mol.uhf)
    except RuntimeError as e:
        if config.general.verbosity > 0:
            print(f"Refinement failed for cycle {cycle + 1}.")
            if config.general.verbosity > 1 or config.refine.debug:
                print(e)
        return None
    finally:
        if config.refine.debug:
            stop_event.set()

jonathan-schoeps and others added 2 commits October 9, 2024 15:08
Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>
src/mindlessgen/qm/xtb.py Outdated Show resolved Hide resolved
…plemented

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
@jonathan-schoeps
Copy link
Contributor Author

Now all the requested changes should be implemented.

src/mindlessgen/qm/xtb.py Outdated Show resolved Hide resolved
Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
src/mindlessgen/qm/xtb.py Outdated Show resolved Hide resolved
Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Copy link
Member

@marcelmbn marcelmbn left a comment

Choose a reason for hiding this comment

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

LGTM.

@jonathan-schoeps jonathan-schoeps merged commit 3ac09a5 into grimme-lab:main Oct 10, 2024
10 checks passed
@jonathan-schoeps jonathan-schoeps deleted the bug/f_in_core_approx branch October 10, 2024 10:55
marcelmbn added a commit that referenced this pull request Oct 10, 2024
* Initial commit contract coordinates

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* new function to contract the coordinates before the xtb scf

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* The coordinates are now drawn towards the origin to improve convergence.

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* The coordinates are now drawn towards the origin to improve convergence.

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* suggest changes to dev/contract_coordinates to make code more consistent and clean

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

* merged dev/dist_cov_radii and the changes to the function contract_coords

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* bug fix were the boolean was not correctly passed on

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* add IP/EA check with g-xTB for special purpose dev application only

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

* rename gp3 to gxtb consistently

Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>

* Changes which were done twice are removed

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Changed a thing left over from teh debugging

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Changed a Todo

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* suggested changes within the PR

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Update mindlessgen.toml

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update src/mindlessgen/prog/config.py

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* mindlessgen.toml now consistent with the default

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Update on ReadMe.md and clearification with the different scaling factors

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Update CHANGELOG.md

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update CHANGELOG.md

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update mindlessgen.toml

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update README.md

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update README.md

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* Update mindlessgen.toml

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* A sentence is now bolt

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Changed the uhf value when xtb calculates with lanthanides (#60)

* Changed the uhf assingement when xtb calculates with lanthanides

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* The f electrons are now calculated and substractet correctly

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* Update CHANGELOG.md

Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

* The f electron removal for lanthanides are now for the singlepoint implemented

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* check_ligand_uhf is now a function

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* The ValueError is now inside the check_ligand_uhf function

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

* check_ligand_uhf now returns nothing but raises a ValueError

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>

---------

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Co-authored-by: Marcel Mueller <marcel.mueller@thch.uni-bonn.de>

---------

Signed-off-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Signed-off-by: Marcel Müller <marcel.mueller@thch.uni-bonn.de>
Co-authored-by: Jonathan Schöps <s6jtscho@uni-bonn.de>
Co-authored-by: Joanthan Schöps <106986430+jonathan-schoeps@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

Wrong UHF assignment for f-in-core methods in xtb
2 participants