Pyberny addition #3

jlheflin · 2024-06-14T17:40:36Z

Is this pull request associated with an issue(s)?
No.

Description
Adds a beginning branch for adding PyBerny as an option for optimizing a chemical system geometry.

TODOs
Implement ModuleManager so that modules can be loaded

…duleManager

CLAassistant · 2024-06-14T17:40:41Z

All committers have signed the CLA.

ryanmrichard

Have you tried building this? Based on what's here, I suspect the build will fail.

Take a look at https://github.com/NWChemEx/FriendZone. In particular, try to mirror its file structure (notably the C++/Python split).

You also need a test for your function see here for an example of how to add one to the build system.

src/structurefinder/PyBernyOpt.py

… to src/python subdir.

… geometry

…nput

make lowercase to avoid problems on case-insensitive filesystems.

I'd add your toolchain to the .gitignore. We've talked about at some point having a repo with some starter toolchains, but for now we're not checking them in to any repos.

src/python/structurefinder/pybernyopt.py

…er the molecule object

Small refactor

jwaldrop107

A couple of comments regarding documentation and workflows.

.github/workflows/pull_request.yaml

CMakeLists.txt

.github/workflows/merge.yaml

…r and pyberny modules

removes redundant file (this function is now merged with the newly created module)

src/python/structurefinder/pyberny/__init__.py

tests/python/unit_tests/test_optimizers/test_pybernyop.py

…urefinder into pyberny_addition

…py file

ryanmrichard · 2024-06-28T20:03:56Z

@jlheflin your current error is because you're depending on friendzone for your module. Modules shouldn't directly depend on other modules/functionality found in other plugins. Instead they should depend on property types. For now, what I would do is write a function which loops over the XYZ you get back from PyBerny and directly fill that in to a ChemicalSystem object.

ryanmrichard · 2024-06-28T20:04:42Z

@jwaldrop107 and I discussed some options for making a more reusable solution, but let's tackle that later.

ryanmrichard · 2024-07-09T03:22:11Z

CMakeLists.txt

+    nwx_pybind11_tests(
+        py_${PROJECT_NAME}
+        "${${PROJECT_NAME}_PYTHON_TEST_DIR}/unit_tests/test_${PROJECT_NAME}.py"
+        SUBMODULES simde chemist pluginplay parallelzone
+    )


Suggested change

nwx_pybind11_tests(

py_${PROJECT_NAME}

"${${PROJECT_NAME}_PYTHON_TEST_DIR}/unit_tests/test_${PROJECT_NAME}.py"

SUBMODULES simde chemist pluginplay parallelzone

)

cmaize_find_or_build_dependency(

nwchemex

URL github.com/NWChemEx/NWChemEx

BUILD_TARGET nwchemex

FIND_TARGET nwx::nwchemex

CMAKE_ARGS BUILD_TESTING=OFF

)

nwx_pybind11_tests(

py_${PROJECT_NAME}

"${${PROJECT_NAME}_PYTHON_TEST_DIR}/unit_tests/test_${PROJECT_NAME}.py"

DEPENDS nwchemex

SUBMODULES simde chemist pluginplay parallelzone friendzone chemcache nwchemex

)

Try this for your CI error. Your unit test depends on NWChemEx (which also means it's technically an integration test) so you need to install NWChemEx and link to it.

ryanmrichard

I'm going to recommend breaking this into two PRs. In PR put all the infrastructure required to get the repo working. Having a library that prints "hello world" and a test checking the result is fine. The second PR then adds the Pyberny stuff. The LJ stuff is presumably taken care of by Felix's PR.

ryanmrichard · 2025-01-20T20:56:18Z

Disregard my last suggestion. One PR is fine. I thought there was more to PyBerny than there is.

jlheflin · 2025-02-07T05:53:59Z

The GitHub Workflow is showing that the full ChemCache is being built, is this due to having chemcache in the CMakeLists.txt for the nwx_pybind11_tests submodules? I'm pretty sure we don't need the full ChemCache to test this.

Edit: I went ahead and stopped the workflow from completing to avoid anything that might get result in a charge. ChemCache already takes forever on my system, I'm sure this would result in a 2 day endeavor.

jwaldrop107 · 2025-02-07T06:17:12Z

The full ChemCache is being built because it is brought in as a dependency of NWChemEx, which defaults to using the generated_data branch. This behavior is control by this CMake option. The simple solution is to add BUILD_FULL_CHEMCACHE=OFF to the find_or_build call for NWX here

jlheflin · 2025-02-07T15:16:44Z

Docs are for boats. Also unsure of how to resolve the SimDE dependency issue.

ryanmrichard · 2025-02-07T15:26:45Z

.github/workflows/pull_request.yaml

+      config_file: '.github/.licenserc.yaml'
+      source_dir: ''
+      compilers: '["gcc-11", "clang-14"]'
+      doc_target: 'structure_finder_cxx_api'


Suggested change

doc_target: 'structure_finder_cxx_api'

doc_target: ''

Try this. You don't have C++ source, so there's no reason to call Doxygen.

Also, the point of git commit messages are to: record what you did, and provide succinct updates to others following the development. If you want to be cheeky that's fine, but try to make them somewhat descriptive.

Thanks. I will also add better commit messages in the future.

ryanmrichard · 2025-02-07T15:27:54Z

CMakeLists.txt

+# Builds C++ API documentation
+include(nwx_cxx_api_docs)
+nwx_cxx_api_docs("${${PROJECT_NAME}_INC_DIR}" "${${PROJECT_NAME}_SRC_DIR}")


Suggested change

# Builds C++ API documentation

include(nwx_cxx_api_docs)

nwx_cxx_api_docs("${${PROJECT_NAME}_INC_DIR}" "${${PROJECT_NAME}_SRC_DIR}")

No C++ docs to build.

ryanmrichard · 2025-02-14T17:06:51Z

What is this PR waiting on?

jlheflin · 2025-02-14T17:11:21Z

I think this is r2g. I double checked the tests and it tests PyBerny and is based on the optimization of H2, and returns the energy. For now it is fine, but I will need to start another pull request to develop a property type that returns an energy and a chemical system with the resulting coordinates.

jwaldrop107 · 2025-02-14T17:13:16Z

🚀 [bumpr] Bumped! New version:v0.0.1

jlheflin added 4 commits June 13, 2024 10:57

Initial commit using cookiecutter to generate plugin template.

c6af382

Added PyBernyOpt.py and __init__.py

313f15d

Removed scripting text at top of PyBernyOpt

4146ec8

Removed structurefinder_mm.cpp and added structurefinder_mm.py for Mo…

68d7614

…duleManager

ryanmrichard reviewed Jun 17, 2024

View reviewed changes

src/structurefinder/PyBernyOpt.py Outdated Show resolved Hide resolved

src/structurefinder/PyBernyOpt.py Outdated Show resolved Hide resolved

jlheflin and others added 6 commits June 18, 2024 09:24

Removed structurefinder_mm.py and moved python module structurefinder…

6e0b3dd

… to src/python subdir.

Mirrored file structure of FriendZone

fca3d17

PyBernyOpt.py now returns the energy as well as the xyz string of the…

e9d44ee

… geometry

Changed wording on self.input in __init__.py to reflect the correct i…

c6a3b71

…nput

Rename PyBernyOpt.py to pybernyopt.py

b3374d8

make lowercase to avoid problems on case-insensitive filesystems.

Delete default_toolchain.cmake

5d0f334

I'd add your toolchain to the .gitignore. We've talked about at some point having a repo with some starter toolchains, but for now we're not checking them in to any repos.

ryanmrichard reviewed Jun 20, 2024

View reviewed changes

src/python/structurefinder/pybernyopt.py Outdated Show resolved Hide resolved

jlheflin and others added 4 commits June 20, 2024 13:43

Made it so that the xyz string that is made is generated by a loop ov…

46e5ace

…er the molecule object

more aligned with nwx

423ec3d

Merge pull request #4 from NWChemEx/refactor

853fc8c

Small refactor

Committing clang-format changes

907b88a

jwaldrop107 reviewed Jun 24, 2024

View reviewed changes

.github/workflows/pull_request.yaml Outdated Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

.github/workflows/merge.yaml Outdated Show resolved Hide resolved

jlheflin and others added 7 commits June 24, 2024 14:30

Restructured to nwx style, added __init__.py files for structurefinde…

4ba4214

…r and pyberny modules

Fixed typo

1fcb570

Committing clang-format changes

5488415

Adjusted __init__.py files to reflect that of FriendZone

8377697

Fixed indentation issue

80542aa

address jwaldrop's comments

0e4b3ca

Delete src/python/structurefinder/pybernyopt.py

1978f02

removes redundant file (this function is now merged with the newly created module)

ryanmrichard reviewed Jun 25, 2024

View reviewed changes

ryanmrichard and others added 4 commits June 25, 2024 09:16

add exception

e1e0524

Merge branch 'pyberny_addition' of https://github.com/nwchemex/struct…

4a86be4

…urefinder into pyberny_addition

Committing clang-format changes

54ba7c9

Made suggested changes to test_pybernyop.py and the pyberny __init__.…

2b63ee8

…py file

jlheflin and others added 5 commits July 8, 2024 13:32

Added diagnostic printing and removed dependency of FriendZone

477e58f

Fixed merge conflicts

591ba2d

Added parameters to allow tests to be passed for PyBerny optimization.

8f420ab

Committing clang-format changes

1746d1b

Adjusted test files

fceb006

ryanmrichard reviewed Jul 9, 2024

View reviewed changes

jlheflin and others added 4 commits July 8, 2024 22:52

Added NWChemEx as a dependency for integration test

84d379d

made changes so StructureFinder can build

8582fac

added LJ Potential Module Directory

8c1b483

Committing clang-format changes

4b1df13

ryanmrichard reviewed Jan 20, 2025

View reviewed changes

jlheflin and others added 2 commits February 6, 2025 23:39

consulted the Oracle on how to use EnergyNuclearGradientStdVectorD

1d3a9b8

Committing clang-format changes

b56b164

me when the Py is Berny

aa6ff96

ryanmrichard reviewed Feb 7, 2025

View reviewed changes

made changes to workflows and CMakeLists.txt for docs

00a20c9

jlheflin marked this pull request as ready for review February 14, 2025 17:11

ryanmrichard approved these changes Feb 14, 2025

View reviewed changes

ryanmrichard merged commit 32e63eb into master Feb 14, 2025
5 checks passed

ryanmrichard deleted the pyberny_addition branch February 14, 2025 17:13

-    nwx_pybind11_tests(
-        py_${PROJECT_NAME}
-        "${${PROJECT_NAME}_PYTHON_TEST_DIR}/unit_tests/test_${PROJECT_NAME}.py"
-        SUBMODULES simde chemist pluginplay parallelzone
-    )
+    cmaize_find_or_build_dependency(
+        nwchemex
+        URL github.com/NWChemEx/NWChemEx
+        BUILD_TARGET nwchemex
+        FIND_TARGET nwx::nwchemex
+        CMAKE_ARGS BUILD_TESTING=OFF
+    )
+    nwx_pybind11_tests(
+        py_${PROJECT_NAME}
+        "${${PROJECT_NAME}_PYTHON_TEST_DIR}/unit_tests/test_${PROJECT_NAME}.py"
+        DEPENDS nwchemex
+        SUBMODULES simde chemist pluginplay parallelzone friendzone chemcache nwchemex
+    )

	# Builds C++ API documentation
	include(nwx_cxx_api_docs)
	nwx_cxx_api_docs("${${PROJECT_NAME}_INC_DIR}" "${${PROJECT_NAME}_SRC_DIR}")

Pyberny addition #3

Pyberny addition #3

Uh oh!

Conversation

jlheflin commented Jun 14, 2024

Uh oh!

CLAassistant commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanmrichard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jwaldrop107 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanmrichard commented Jun 28, 2024

Uh oh!

ryanmrichard commented Jun 28, 2024

Uh oh!

ryanmrichard Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

ryanmrichard left a comment

Choose a reason for hiding this comment

Uh oh!

ryanmrichard commented Jan 20, 2025

Uh oh!

jlheflin commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jwaldrop107 commented Feb 7, 2025

Uh oh!

jlheflin commented Feb 7, 2025

Uh oh!

ryanmrichard Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

jlheflin Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

ryanmrichard Feb 7, 2025

Choose a reason for hiding this comment

Uh oh!

ryanmrichard commented Feb 14, 2025

Uh oh!

jlheflin commented Feb 14, 2025

Uh oh!

Uh oh!

jwaldrop107 commented Feb 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Jun 14, 2024 •

edited

Loading

jlheflin commented Feb 7, 2025 •

edited

Loading