Pymole feature add 1d operators - #463
Conversation
…s for 1D, 2D, and 3D dimensions
…iated unit tests and CI configuration.
…n, and include Python dependencies file
…ccompanying tests
…ponding unit tests
…nd consistency with Divergence and Gradient
…ndition classes for finite difference modeling
…csrc-sdsu/mole into pymole-feature-add-1d-operators
…y condition example
Signed-off-by: Christopher Paolini <paolini@engineering.sdsu.edu>
Added README.md for pymole Python interface with installation and usage instructions. Signed-off-by: Christopher Paolini <paolini@engineering.sdsu.edu>
Signed-off-by: Christopher Paolini <paolini@engineering.sdsu.edu>
Tony-Drummond
left a comment
There was a problem hiding this comment.
The instructions in the README.md now work as expected. All the tests run and also the example.
@aboada I did not suggest to use /src, if you read carefully my comment suggests At first this organization may suggest that other python implementations of MOLE could co-exist under the python subdirectory, which is a good design consideration for future extensibility. which is very different from how you interpreted. Let me expand. According to the Python Enhancement Proposals, PEP 518 and PEP 621, PEPs for minimal package requirements and storing the project metadata, respectively, the following scenario is possible: The above requires special handling to accommodate for build tools looking for a single pyproject.toml file (e.g., have a script that the user can set one of the *.toml files as the package to install for pip, build. Alternatively, use tomlib to build one package at a time, etc.) Another, possibility is to adopt this directory structure: Then users can cd projectX and pip install . (or python -m build, etc.) and all projects are independently buildable with zero custom tooling. I hope the details above helps. Definitively, I never intended to suggest the use a single /src for multiple python implementations of MOLE. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…metic operator modules
|
@aboada Used Ruff to apply formatting to elliptic1D.py. |
|
@cpaolini Since this PR has had a lot of activity, I am summarizing here all the changes that you have already implemented:
I believe that the unchecked items above is all we need before merging this PR. The current deadline is before Thursday this week so it can be included with the MOLE 1.3.0 release. |
My concern really is a clear separation of concerns between source, examples and tests. It seems you are onboard with that so we're aligned. I don't disagree with what you are saying. The potential approaches you are suggesting are fine by me as long as we respect the mentioned separation of concerns. |
|
Thanks for the summary, @Tony-Drummond. From my end:
I think there was value in this file. Perhaps it can be added as an integration test of some sort in the future. No issues with the removal.
Not a blocker for me, but having .gitkeep files in folders with checked-in files provides no value and pollutes the codebase. I still suggest removal.
I would want to know if there was an issue created for it. If not, we should create one. It should be easy to write this example in a Pythonic way, which will provide value for the library.
As stated in my comment, consistency is key. Moreover, we should adhere to PEP8 recommendation for module (file) names. I think the only blocker for me is the filenames. They should be all |
…BoundaryCondition
…itions, and mimetic operators
|
@aboada The files BoundaryCondition.py and RobinBoundaryCondition.py were renamed to use snake_case, becoming boundary_condition.py and robin_boundary_condition.py. |
|
@aboada All six .gitkeep files have been removed. |
…itions, and grid implementations
…ndary conditions with an example model problem
…itions, and examples
|
Hi! I really appreciate this contribution and all of the reviewer's work. However, @Tony-Drummond , please in the future refrain from merging such a long history of commits in the |
What type of PR is this? (check all applicable)
Description
The pymole-feature-add-1d-operators branch introduces an initial Python implementation of MOLE’s mimetic operators. This branch features an installable pymole package, multidimensional grid generation, partial 2-D/3-D support, boundary-condition infrastructure, tests, examples, documentation, and Python CI. Classes include a 1-D, 2-D, and 3-D Grid; 1-D, 2-D, and partial 3-D Gradient; 1-D and 2-D Divergence; 1-D and 2-D Laplacian; 2-D Curl; and RobinBoundaryCondition. The branch adds 47 Python unit tests that cover: Grid construction, spacing, dimensions, validation, and unpacking; Operator matrix dimensions; Rejection of unsupported accuracy orders or undersized grids; Zero derivatives of constant fields; Basic linear and quadratic-field behavior; The identity L=DG and the basic 2-D curl behavior are included. The updated ci.yml runs these tests on Python 3.10, 3.11, and 3.12. A separate Python documentation workflow builds Sphinx documentation and deploys the generated HTML on pushes. A 1D Poisson's equation example is provided.
Related Issues & Documents
QA Instructions, Screenshots, Recordings
$ pytest python/tests
============================= test session starts ==============================
platform linux -- Python 3.10.21, pytest-9.1.1, pluggy-1.6.0
rootdir: /home/runner/work/mole/mole/python
configfile: pyproject.toml
collected 47 items
python/tests/test_curl.py ... [ 6%]
python/tests/test_divergence.py ....... [ 21%]
python/tests/test_gradient.py ............. [ 48%]
python/tests/test_grid_utility.py ................ [ 82%]
python/tests/test_laplacian.py ........ [100%]
============================== 47 passed in 0.42s ==============================
Keep-open request
keep-open.Reason:
Added/updated tests?
_We encourage you to test all code included with MOLE, including examples.
have not been included
Read Contributing Guide and Code of Conduct
[optional] Are there any post deployment tasks we need to perform?
No
[optional] What gif best describes this PR or how it makes you feel?