Python BCs #2170

chleh · 2018-07-31T14:03:00Z

Python code for BCs

Limitations (not solved in this PR)

Only works if all physical fields have the same ansatz functions, i.e., not for HM.
The normal traction BC is special and not covered by this PR.
Constraint boundary conditions (First version of constraint boundary conditions #2145) cope with secondary variables (fluxes). Fluxes cannot be passed to the Python scripts by the code of this PR.

TODO

finish Hertz contact test
test with nonlinear BC
test with multi-variable process
input file documentation
maybe fix possible Python version mismatch (presumably between VTK and OGS)
fix git lfs tricks (*.pnK)
squash

Review

attribute used
VTK Python version mismatch
discuss NLSolver changes.
Large tests & Python?

Issues to open

Maybe the two versions of deriveBCMap could be merged into one?
Python BC for normal traction
Python BC for mixed FEM ansatz functions
passing secondary variables to Python BC

~~Don't review yet! Soon: review commit-wise.~~

endJunction · 2018-08-02T07:07:50Z

I'm afraid to ask, but 132kloc?

chleh · 2018-08-02T07:37:15Z

Yes. I copied OGS, and Python (2 and 3), and combined the two, and now there are Python BCs. 🤣
No. I assume its due to WIP ParaView state files. I'll remove them. Maybe I'll keep some of them to simplify the validation of (changed) reference solutions.

endJunction · 2018-08-08T09:04:56Z

ProcessLib/CMakeLists.txt

+        # snprintf might be defined even though it's not needed. Precompiled headers
+        # cause that this macro definition is visible in all of ProcessLib.
+        # That might lead to clashes with jsoncpp. In particular the following
+        # compilation error was observed on MSVC:


I could imagine that msvc version and python version would be helpful...

Issue solved. Code and comment removed.

endJunction · 2018-08-13T10:46:26Z

ProcessLib/BoundaryCondition/PythonBoundaryCondition.cpp

+                "script.");
+            return;
+        }
+        if (val.first)


if (!val,first) return ✅

endJunction · 2018-08-13T10:48:05Z

ProcessLib/BoundaryCondition/PythonBoundaryCondition.cpp

+        // gather primary variable values
+        primary_variables.clear();
+        auto const num_var = _dof_table_boundary->getNumberOfVariables();
+        for (int var = 0; var < num_var; ++var)


check for HM ✅

endJunction · 2018-08-13T11:05:09Z

Applications/CLI/CMakeLists.txt

+
+    add_library(ogs_python_bindings STATIC ogs_python_bindings.cpp)
+    target_link_libraries(ogs_python_bindings PRIVATE pybind11::embed)
+    target_include_directories(ogs_python_bindings PRIVATE ${PYTHON_INCLUDE_DIRS})


Cleaned up.

endJunction · 2018-08-13T11:05:46Z

Applications/CLI/CMakeLists.txt

+    target_link_libraries(ogs PRIVATE pybind11::embed)
+
+    add_library(ogs_python_bindings STATIC ogs_python_bindings.cpp)
+    target_link_libraries(ogs_python_bindings PRIVATE pybind11::embed)


Consider PUBLIC.

Cleaned up.

endJunction · 2018-08-14T08:46:10Z

Applications/CLI/ogs_embedded_python.cpp

+    // OpenGeoSys Python module. The name is generated by pybind11. If it is not
+    // obvious that this symbol is actually used, the linker might remove it
+    // under certain circumstances.
+    mark_used(&pybind11_init_impl_OpenGeoSys);


just an idea...
Maybe declaring the pybind11_init_impl_OpenGeoSys function with __attribute__((used)) helps (at least for gcc and clang).

__attribute__(used) is gcc-specific. I didn't find the same for MSVC:
https://msdn.microsoft.com/de-de/library/74f4886t.aspx

Maybe __declspec(dllexport) might help?
https://msdn.microsoft.com/de-de/library/3y1sfaz2.aspx
But is that also applicable for static libs?

Anyway, we'd have to distinguish between different compilers, which is also kind of hackish...

endJunction · 2018-08-14T08:48:40Z

NumLib/DOF/LocalToGlobalIndexMap.cpp

+
+    for (int i = 0; i < getNumberOfComponents(); ++i)
+    {
+        global_component_ids.push_back(i);


~~std::iota(begin(global_component_ids), end(global_component_ids), getNumberOfComponents())~~
Ups...

A "slightly" verbose version would be something like

std::generate_n(std::back_inserter(global_component_ids), getNumberOfComponents(), [n = 0] () mutable { return n++; });

Nevermind

Indeed, verbose 😄

endJunction · 2018-08-14T08:53:01Z

NumLib/DOF/LocalToGlobalIndexMap.cpp

+}
+
+std::unique_ptr<LocalToGlobalIndexMap>
+LocalToGlobalIndexMap::deriveBoundaryConstrainedMap(


Maybe the two versions of deriveBCMap could be merged into one?

I'll open an issue.

endJunction · 2018-08-14T09:48:03Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryConditionLocalAssembler.h

+                "bulk_node_ids");
+
+        // gather primary variables
+        Eigen::MatrixXd primary_variables_mat(num_nodes, num_comp_total);


Optionally use compile time size for nodes' size...

Hm... I think, I'd like to leave it all dynamic:
Eigen::Matrix<double, ShapeFunction::size /*?*/, Eigen::Dynamic> doesn't really look more pleasant.

endJunction · 2018-08-14T09:53:44Z

Tests/Data/Mechanics/Linear/Python/gen-unit-circle.py

+    # self.GetOutputDataObject(0).GetPointData().AddArray(r2)
+    # self.GetOutputDataObject(0).GetPointData().AddArray(c2)
+
+    # self.GetOutputDataObject(0).SetPoints(pts) #.SetData(c2)


Maybe some debug comments could be removed...

removed by now.

endJunction

Maybe you could open few issues for the remaining modifications after/close to the merge.

Especially one for the deriveBCMap unification...

wenqing

👍

wenqing · 2018-08-14T12:40:11Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryConditionLocalAssembler.h

+                for (unsigned element_node_id = 0; element_node_id < num_nodes;
+                     ++element_node_id)
+                {
+                    auto* node = _element.getNode(element_node_id);


may be auto const* const node

wenqing · 2018-08-14T12:59:16Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryConditionLocalAssembler.h

+                        _data.dof_table_bulk.getGlobalIndex(loc, var, comp);
+                    if (dof_idx == NumLib::MeshComponentMap::nop)
+                    {
+                        // TODO extend Python BC to mixed FEM


So far we do not have mixed FEM implemented. Maybe it can be said as "extend Python BC to processes using Taylor-Hood elements".

// TODO extend Python BC to mixed FEM ansatz functions still fits on a single line (and therefore is displayed completely in QtCreator's todo list). I'd suggest that wording.

wenqing · 2018-08-14T13:21:28Z

web/content/docs/benchmarks/python-bc/hertz-contact/hertz-contact.md

+values in every load step.
+Due to symmetry reasons a flat circular contact area of radius $a$ forms.
+
+{{< img src="../hertz-contact.pnK" >}}


Figures in this documentation are not displayed (see https://deploy-preview-2170--ogs.netlify.com/docs/benchmarks/python-bc/hertz-contact/hertz-contact/).

The file names now are fixed.

bilke · 2018-08-14T14:21:11Z

Regarding VTK Python version mismatch:

In your VTK installation there is a file lib/cmake/vtk-8.1/VTKConfig.cmake. Can you please check if there is some info about Python version. If yes (e.g. in the variable VTK_PYTHON_VERSION) you can write a check in our CMake similar to this:

if (NOT ${VTK_PYTHON_VERSION} VERSION_EQUAL ${PYTHON_VERSION_STRING})
...
endif()

TomFischer · 2018-08-15T09:21:29Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryCondition.cpp

+        if (!_flush)
+            return;
+
+        using namespace pybind11::literals;


I'm not familiar with pybind11 and I wonder why the namespace here is needed?

For the line below, e.g. "end"_a is the equivalent to a Python named argument.

TomFischer · 2018-08-15T09:22:10Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryCondition.cpp

+            return;
+
+        using namespace pybind11::literals;
+        pybind11::print("end"_a = "", "flush"_a = true);


Could you explain this line, please?

It's equivalent to (Python) print(end="", flush=True), i.e., no end-of-line will be printed and output will be flushed.
So the behaviour is described in the docu of the destructor. Is that sufficient?

Thanks for the explanation. Is my understanding correct that _a means that the previous characters is a python argument name?

Yes. _a is a user-defined literal.
There are also other such literals, e.g. s in C++14.
E.g. in

using namespace std::string_literals; auto str = "test"s;

str is a std::string, not a character array.

bilke

There is still a ParaView state file in this PR. Is it intended to keep? Other than that, very good work!

bilke · 2018-08-15T09:35:47Z

ProcessLib/BoundaryCondition/Python/CMakeLists.txt

+    PUBLIC BaseLib MathLib MeshLib NumLib logog)
+
+target_link_libraries(ProcessLibBoundaryConditionPython
+    PRIVATE pybind11::pybind11)


You can merge this command with the previous one:

target_link_libraries(ProcessLibBoundaryConditionPython PUBLIC BaseLib MathLib MeshLib NumLib logog PRIVATE pybind11::pybind11)

All pvsm files will be deleted.
Thanks for the hint. I'll change it.

TomFischer · 2018-08-15T09:55:06Z

ProcessLib/BoundaryCondition/Python/PythonBoundaryCondition.cpp

+                                      MeshLib::MeshItemType::Node,
+                                      bulk_node_id};
+                auto const dof_idx =
+                    _bc_data.dof_table_bulk.getGlobalIndex(loc, var, comp);


Is it possible to compute the dof_idx also from the boundary dof table?

I think, it's not. I think I tried, but the dof table for the boundary is for a hypothetical equation system that is assembled solely on the boundary (i.e., with no connection to the bulk). Therefore all boundary dof indices would be wrong. @endJunction, is that right? If unsure, I can also check that.

TomFischer · 2018-08-15T11:00:12Z

Tests/Data/Mechanics/Linear/Python/gen-unit-circle.py

+a = np.empty(coords.shape[0])
+
+for i, (x, y, z) in enumerate(coords):
+    r = np.sqrt(x**2 + y**2)


Where is r used?

In some old version of this script, long, long time ago. I'll remove it.

TomFischer

Looks good. 👍

chleh · 2018-08-17T07:56:59Z

I just rebased and squashed this PR. Thank you for the quick reviewing process and for your help.
Lately, netlify failed because hugo could not resolve some page reference. Locally, at my machine it works, however. @bilke, could you please have a look.

Unfortunately I had to exclude the Hertz contact test, because it requires a non-trivial change to the non-linear solver, cf. #2184.

Remaining for this PR are:

I decided not to use __attribute__(used) because it's not portable and I don't know of a version that works for MSVC.
The Elder test case is a LARGE one. Are LARGE tests tested with a Python build?

If this PR builds successfully, from my perspective it's ready to be merged if you agree.

endJunction · 2018-08-17T09:41:10Z

LARGE tests are not tested.

bilke · 2018-08-17T12:15:53Z

For the failing web build merge this: chleh#10

[web] Updated Hugo to 0.47.

ogsbot · 2020-06-19T12:53:45Z

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

chleh added the WIP 🏗️ label Jul 31, 2018

chleh force-pushed the python-bcs-for-pr branch 3 times, most recently from 2cced46 to ca03670 Compare August 7, 2018 17:26

endJunction reviewed Aug 8, 2018

View reviewed changes

chleh force-pushed the python-bcs-for-pr branch from 1a1e493 to f0ca87c Compare August 8, 2018 12:26

endJunction reviewed Aug 13, 2018

View reviewed changes

chleh force-pushed the python-bcs-for-pr branch from 9aaea6a to f925e71 Compare August 13, 2018 13:55

endJunction reviewed Aug 14, 2018

View reviewed changes

endJunction approved these changes Aug 14, 2018

View reviewed changes

wenqing approved these changes Aug 14, 2018

View reviewed changes

wenqing reviewed Aug 14, 2018

View reviewed changes

TomFischer reviewed Aug 15, 2018

View reviewed changes

bilke approved these changes Aug 15, 2018

View reviewed changes

TomFischer reviewed Aug 15, 2018

View reviewed changes

TomFischer approved these changes Aug 15, 2018

View reviewed changes

ufz deleted a comment from bilke Aug 15, 2018

chleh added 16 commits August 17, 2018 08:36

[PL] extended BC interface

1014529

[PL] applied interface changes to existing BCs

5ece875

Introducing Python to OGS.

25d130d

[PL] added Python BC classes

eb93647

[PL] added Python build dependencies

2e47acb

[PL] create Python BCs and data member name improvement

7b32332

Laplace Eq. test and docu

4376add

Added pybind11 submodule

24fca4f

use pybind11 from git module

3c88394

Build with Python on Jenkins

d854226

[Doc] added input file documentation

6b1a321

[App] renamed OGS embedded Python module source files

ca86963

[AppL] improved parsing and restructured CMakeLists

704adf4

[PL] extracted Python BC cmake target

0c3995c

Check Python version mismatch, cleanup definitions

70166cb

Added Elder test case

8a86d4e

chleh force-pushed the python-bcs-for-pr branch from bf64d94 to 7007188 Compare August 17, 2018 07:50

chleh mentioned this pull request Aug 17, 2018

Python BCs: Hertz Contact test #2184

Merged

1 task

chleh added 2 commits August 17, 2018 13:30

Added Piston test case

ed0990f

Added submenu to Hugo

7eda63e

chleh force-pushed the python-bcs-for-pr branch from cd5dbf1 to 7eda63e Compare August 17, 2018 11:30

[web] Updated Hugo to 0.47.

f0da276

Merge pull request #10 from bilke/fix-netlify

0b75840

[web] Updated Hugo to 0.47.

chleh removed the WIP 🏗️ label Aug 17, 2018

endJunction merged commit 3b1cb39 into ufz:master Aug 17, 2018

chleh deleted the python-bcs-for-pr branch August 18, 2018 10:57

Python BCs #2170

Python BCs #2170

Conversation

chleh commented Jul 31, 2018 • edited Loading

Python code for BCs

Limitations (not solved in this PR)

TODO

Review

Issues to open

endJunction commented Aug 2, 2018

chleh commented Aug 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endJunction Aug 13, 2018 • edited by chleh Loading

Choose a reason for hiding this comment

endJunction Aug 13, 2018 • edited by chleh Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endJunction Aug 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endJunction left a comment

Choose a reason for hiding this comment

wenqing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chleh Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bilke commented Aug 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chleh Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bilke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomFischer left a comment

Choose a reason for hiding this comment

chleh commented Aug 17, 2018

endJunction commented Aug 17, 2018

bilke commented Aug 17, 2018

ogsbot commented Jun 19, 2020

chleh commented Jul 31, 2018 •

edited

Loading

endJunction Aug 13, 2018 •

edited by chleh

Loading

endJunction Aug 13, 2018 •

edited by chleh

Loading

endJunction Aug 14, 2018 •

edited

Loading

chleh Aug 15, 2018 •

edited

Loading

chleh Aug 15, 2018 •

edited

Loading