openff-recharge icon indicating copy to clipboard operation
openff-recharge copied to clipboard

Feature/multiprocessing for esp rebuild

Open bismuthadams1 opened this issue 1 year ago • 2 comments

Description

Updated the cli tool for reconstructing the ESP from orbitals and eigenvalues.

Todos

Notable points that this PR has either accomplished or will accomplish.

  • [x] Updated the reconstruct.py cli script by removing the redundant keywordset from qcportal and replacing with a dictionary indexing scheme
  • [x] Switched the multiprocessing in reconstruct.py from fork() to spawn() type, where the fork() version would hang indefinitely. Switch to spawn() means that threads can be copied unlike fork(). Some initial slowdown expected when spawning the processes, but after that it should be about the same speed.
  • [x] Rewrote the test for the cli to capture any future issues with the multiprocessor, mocker the retrieve_result_records and the _process_result function.

Questions

  • [ ] Question1

Status

  • [x] Ready to go

bismuthadams1 avatar Aug 09 '24 07:08 bismuthadams1

Codecov Report

Attention: Patch coverage is 31.81818% with 15 lines in your changes missing coverage. Please review.

Project coverage is 90.89%. Comparing base (e7350c9) to head (9b2dc96). Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
openff/recharge/cli/reconstruct.py 27.27% 8 Missing :warning:
openff/recharge/esp/storage/_storage.py 36.36% 7 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (e7350c9) and HEAD (9b2dc96). Click for more details.

HEAD has 149 uploads less than BASE
Flag BASE (e7350c9) HEAD (9b2dc96)
151 2
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #173      +/-   ##
==========================================
- Coverage   98.29%   90.89%   -7.41%     
==========================================
  Files          38       38              
  Lines        2056     2064       +8     
==========================================
- Hits         2021     1876     -145     
- Misses         35      188     +153     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Aug 09 '24 07:08 codecov[bot]

No pressure, but LMK if/when this is complete or if there are blockers I can try to knock down. I'm only moderately familiar with this codebase but I can offer my best effort

mattwthompson avatar Sep 30 '24 15:09 mattwthompson

I'm seeing this failing test in CI (#177), which I can also reproduce locally. It's clearly in the middle of a complex call stack which is hard for me to reason through. But I suspect it doesn't require a major code change to fix. We might be able to get you an OpenEye license if needed. Any idea @bismuthadams1?

$ python -m pytest --lf                     10:44:12  ☁  feature/multiprocessing_for_esp_rebuild ☂
======================================= test session starts =======================================
platform darwin -- Python 3.11.10, pytest-8.3.3, pluggy-1.5.0
rootdir: /Users/mattthompson/software/openff-recharge
plugins: cov-5.0.0, xdist-3.6.1
collected 1 item
run-last-failure: rerun previous 1 failure (skipped 18 files)

openff/recharge/_tests/cli/test_reconstruct.py F                                            [100%]

============================================ FAILURES =============================================
________________________________________ test_reconstruct _________________________________________
concurrent.futures.process._RemoteTraceback:
"""
Traceback (most recent call last):
  File "/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/concurrent/futures/process.py", line 261, in _process_worker
    r = call_item.fn(*call_item.args, **call_item.kwargs)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'MoleculeESPRecord' object is not callable
"""

The above exception was the direct cause of the following exception:

runner = <click.testing.CliRunner object at 0x148cd23d0>
monkeypatch = <_pytest.monkeypatch.MonkeyPatch object at 0x148cd2c10>

    @requires_openeye
    def test_reconstruct(runner, monkeypatch):
        pytest.importorskip("psi4")

        # mock the process result function
        def mock_retrieve_result_records(record_ids: int = 1) -> tuple[tuple, list[dict]]:
            return [MOCK_QC_RESULT]

        monkeypatch.setattr(
            "openff.recharge.cli.reconstruct._retrieve_result_records",
            mock_retrieve_result_records,
        )
        monkeypatch.setattr(
            "openff.recharge.cli.reconstruct._process_result", mock_process_result
        )

        # Create a mock set of inputs.
        with open("record-ids.json", "w") as file:
            json.dump(["1"], file)

        with open("grid-settings.json", "w") as file:
            file.write(LatticeGridSettings(spacing=1.0).json())

        result = runner.invoke(
            reconstruct_cli,
            "--record-ids record-ids.json --grid-settings grid-settings.json",
        )

        if result.exit_code != 0:
>           raise result.exception

/Users/mattthompson/software/openff-recharge/openff/recharge/_tests/cli/test_reconstruct.py:94:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/site-packages/click/testing.py:408: in invoke
    return_value = cli.main(args=args or (), prog_name=prog_name, **extra)
/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/site-packages/click/core.py:1078: in main
    rv = self.invoke(ctx)
/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/site-packages/click/core.py:1434: in invoke
    return ctx.invoke(self.callback, **ctx.params)
/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/site-packages/click/core.py:783: in invoke
    return __callback(*args, **kwargs)
/Users/mattthompson/software/openff-recharge/openff/recharge/cli/reconstruct.py:128: in reconstruct
    esp_record = future.result()
/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/concurrent/futures/_base.py:449: in result
    return self.__get_result()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = None

    def __get_result(self):
        if self._exception:
            try:
>               raise self._exception
E               TypeError: 'MoleculeESPRecord' object is not callable

/Users/mattthompson/micromamba/envs/openff-recharge-test/lib/python3.11/concurrent/futures/_base.py:401: TypeError
===================================== short test summary info =====================================
FAILED openff/recharge/_tests/cli/test_reconstruct.py::test_reconstruct - TypeError: 'MoleculeESPRecord' object is not callable
======================================== 1 failed in 3.60s ========================================

mattwthompson avatar Oct 22 '24 15:10 mattwthompson

#177

mattwthompson avatar Jan 07 '25 15:01 mattwthompson