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

Python source terms #2234

Merged
merged 12 commits into from Oct 26, 2018

Conversation

Projects
None yet
3 participants
@TomFischer
Copy link
Member

TomFischer commented Oct 4, 2018

A first version of Python source term implementation.

ToDo:

  • Check constness, final, forward declarations, ...
  • Add a more complicated example than constant source terms.
  • Documentation of project file tags.
  • Website documentation.

Thanks to @chleh for the support!

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch 2 times, most recently from 7635fd3 to 44ba88e Oct 9, 2018

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch 2 times, most recently from e9552e7 to 901db7b Oct 10, 2018

@@ -189,7 +189,7 @@ void Process::assemble(const double t, GlobalVector const& x, GlobalMatrix& M,
(_coupled_solutions) != nullptr ? _coupled_solutions->process_id : 0;
_boundary_conditions[pcs_id].applyNaturalBC(t, x, K, b, nullptr);

_source_term_collections[pcs_id].integrate(t, b);
_source_term_collections[pcs_id].integrate(t, x, b, nullptr);

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

Could you add a comment to this and foregoing nullptr, please? Something like /* unused jacobian */

#include "CreatePythonSourceTerm.h"

#include <pybind11/pybind11.h>
#include <iostream>

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

Logog for cout... Maybe you need it for flushing, though.

This comment has been minimized.

@TomFischer

TomFischer Oct 11, 2018

Author Member

Yes, it is needed for flushing to keep the C++ output and the python output in the correct sequence.

if (!flush)
return;

LOGOG_COUT << std::flush;

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

std::flush is in <ostream>...

This comment has been minimized.

@TomFischer

TomFischer Oct 12, 2018

Author Member

Since the macro LOGOG_COUT is defined as #define LOGOG_COUT std::cout in ThirdParty/logog/include/string.hpp:39 it is necessary to include iostream.

//! Optionally flushes the standard output upon creation and destruction.
//! Can be used to improve the debug output readability when printing debug
//! messages both from OGS and from Python.
class FlushStdoutGuard

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

final, just for safety

}
catch (MethodNotOverriddenInDerivedClassException const& /*e*/)
{
DBUG("Method `getFlux' not overridden in Python script.");

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

I would either call OGS_FATAL or rethrow the exception, since the whole class' purpose is to call that getFlux...

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

I might be completely wrong here....

This comment has been minimized.

@chleh

chleh Oct 10, 2018

Collaborator

I think MethodNotOverriddenInDerivedClassException won't be thrown anyway, b/c @TomFischer removed that "feature".

This comment has been minimized.

@TomFischer

TomFischer Oct 11, 2018

Author Member

Will remove the try-catch block.

#pragma once

#include "NumLib/DOF/LocalToGlobalIndexMap.h"
#include "NumLib/IndexValueVector.h"

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

Looks like this particular one is not needed. Can you try to put some of the includes into cpp and use fwd decls here?
These should be possible to put as fwd decls:
PythonSourceTermPythonSideInterface
LocalToGlobalIndexMap

unsigned const num_integration_points =
_integration_method.getNumberOfPoints();
auto const num_var = _data.dof_table_bulk.getNumberOfVariables();
auto const num_nodes = _element.getNumberOfNodes();

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

Use ShapeFunction::NPOINTS.

}
}

Eigen::VectorXd local_rhs = Eigen::VectorXd::Zero(num_nodes);

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

NodalVectorType, or does it have to be dynamic for the python bindings?


for (unsigned ip = 0; ip < num_integration_points; ip++)
{
auto const ip_data = _ip_data[ip];

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

should be a ref, not a copy. Same for the following definitions in the for-loop scope.

initial and boundary conditions. It also references the bulk mesh and the
boundary meshes associated with the bulk mesh. The source term $Q$ is defined
in the Python script
`Elliptic/square_1x1_GroundWaterFlow_Python/sinx_siny_source_term.py`. The

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

I'd add links to the repo, so the files are downloadable.

```

The Python script
`Elliptic/square_1x1_GroundWaterFlow_Python/sin_x_sin_y_source_term.py` is

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

sin_x_sin_y vs. sinx_siny above... Links would resolve the issue...


The Python script
`Elliptic/square_1x1_GroundWaterFlow_Python/sin_x_sin_y_source_term.py` is
```

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

If you add python after the three backticks, would it then do the syntax highlighting?


## Results and evaluation

### Comparison of the numerical and analytical solution

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

solutions

to the prescribed boundary conditions.
{{< img src="../square_1e3_poisson_sin_x_sin_y_sourceterm_Diff_Pressure_AnalyticalSolution_PythonSourceTerm.png" >}}
Since a coarse mesh ($32 \times 32$ elements) is used for the simulation the
difference between the numerical and the analytical solution is relative large.

This comment has been minimized.

@endJunction

endJunction Oct 10, 2018

Member

relatively

@TomFischer TomFischer added the WIP 🏗 label Oct 10, 2018

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch 2 times, most recently from af20ef2 to 0cba195 Oct 11, 2018

@TomFischer TomFischer removed the WIP 🏗 label Oct 12, 2018

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch from c4c485b to ddfafe6 Oct 12, 2018

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch 2 times, most recently from 1b73b27 to 2630526 Oct 23, 2018

int const component_id, unsigned const integration_order,
unsigned const shapefunction_order, unsigned const global_dim)
{
std::cout << "createPythonSourceTerm" << std::endl;

This comment has been minimized.

@endJunction

endJunction Oct 26, 2018

Member

INFO, DBUG? Then iostream is not needed in this file

@TomFischer TomFischer force-pushed the TomFischer:PythonSourceTerms branch from 2630526 to 9bb1531 Oct 26, 2018

@TomFischer TomFischer merged commit 50c3582 into ufz:master Oct 26, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details

@TomFischer TomFischer deleted the TomFischer:PythonSourceTerms branch Oct 26, 2018

@TomFischer

This comment has been minimized.

Copy link
Member Author

TomFischer commented Oct 26, 2018

Thanks @chleh for the support!
Thanks @chleh and @endJunction for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.