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

Implementation of nodal source terms #1977

Merged
merged 12 commits into from Nov 9, 2017

Conversation

Projects
None yet
4 participants
@TomFischer
Member

TomFischer commented Nov 2, 2017

Nodal source terms are setup in the project file in the process_variable section.

<source_terms>
    <source_term>
        <geometrical_set>geometry</geometrical_set>
        <geometry>inner</geometry>
        <type>Nodal</type>
        <value>1</value>
    </source_term>
</source_terms>

@TomFischer TomFischer force-pushed the TomFischer:NodalSourceTerms branch from 03b5507 to 48c5e83 Nov 2, 2017

@@ -0,0 +1,4 @@
The name of the geometrical set (see \ref ogs_file_param__gml__name) in which
the \ref
ogs_file_param__prj__process_variables__process_variable__source_terms__ource_term_geometry

This comment has been minimized.

@endJunction

endJunction Nov 2, 2017

Member

ource_term_geometry -> source_term_geometry I guess...

@@ -20,6 +20,7 @@
namespace MeshGeoToolsLib
{
inline
std::unique_ptr<MeshGeoToolsLib::SearchLength> createSearchLengthAlgorithm(
BaseLib::ConfigTree const& external_config, MeshLib::Mesh const& mesh)
{

This comment has been minimized.

@endJunction

endJunction Nov 2, 2017

Member

Would you move it to cpp, please.

@@ -0,0 +1,98 @@
AddTest(

This comment has been minimized.

@endJunction

endJunction Nov 2, 2017

Member

I'd move the Tests.cmake content to the corresponding process' Tests.cmake....

@endJunction

You have done extensive tests, but there is no comparison, which would be nice I think.
Few documentation comments should be fixed.

#include "MeshGeoToolsLib/HeuristicSearchLength.h"
#include "MeshGeoToolsLib/SearchLength.h"
namespace MeshLib

This comment has been minimized.

@endJunction

endJunction Nov 3, 2017

Member

Yes. This is much nicer because of the removed includes and the fwd-decls.

@@ -0,0 +1,4 @@
The name of the geometrical set (see \ref ogs_file_param__gml__name) in which
the \ref
ogs_file_param__prj__process_variables__process_variable__source_terms__source_term_geometry

This comment has been minimized.

@endJunction

endJunction Nov 3, 2017

Member

underscore missing before geometry: ...__source_term__geometry

config.checkConfigParameter("type", "Nodal");
//! \ogs_file_param{prj__process_variables__process_variable__source_terms__source_term__value}
auto const nodal_value = config.getConfigParameter<double>("value");

This comment has been minimized.

@endJunction

endJunction Nov 3, 2017

Member

The <value> tag belongs to the "Nodal" type, so the ogs_file_param must be ..._source_term__Nodal__value. And then you need a new folder Nodal/ in the Documentation/.../source_term/. Compare to DirichletBC.cpp

@wenqing

wenqing approved these changes Nov 3, 2017

👍

@TomFischer TomFischer force-pushed the TomFischer:NodalSourceTerms branch from d2b009f to 2a95151 Nov 6, 2017

@endJunction

This comment has been minimized.

Member

endJunction commented Nov 6, 2017

I think you didn't use 'git lfs' properly. Please enable the pre-commit hook.

@TomFischer

This comment has been minimized.

Member

TomFischer commented Nov 7, 2017

If there are no objections I'd like to rebase and merge the branch.

@wenqing

This comment has been minimized.

Member

wenqing commented Nov 7, 2017

if you do not wait jenkins test works

MeshLib::Location const l{_mesh_id, MeshLib::MeshItemType::Node, _node_id};
auto const index =
_dof_table.getGlobalIndex(l, _variable_id, _component_id);
b.add(index, _value);

This comment has been minimized.

@chleh

chleh Nov 8, 2017

Contributor

Does that mean that such source terms only work for single nodes? Is an extension to line sources/volumetric sources planned?

This comment has been minimized.

@TomFischer

TomFischer Nov 8, 2017

Member

Yes to both questions.

ids.size(), variable_id, *config.component_id);
return ProcessLib::createNodalSourceTerm(
config.config, dof_table, mesh.getID(), ids[0], variable_id,

This comment has been minimized.

@chleh

chleh Nov 8, 2017

Contributor

IMHO it's very bad to silently drop n-1 of n sound node ids.
More complying with our current standards would be to OGS_FATAL if ids.size() > 1.

$$
G(x, y) = \frac{1}{2 \pi} \ln \sqrt{(x-\xi)^2 + (y-\eta)^2}.
$$
With a nodal source term at $(0.0, 0.0)$ the analytical solution is

This comment has been minimized.

@chleh

chleh Nov 8, 2017

Contributor

How strong is the source? Unity?

@TomFischer TomFischer force-pushed the TomFischer:NodalSourceTerms branch from 2a95151 to c4018dc Nov 8, 2017

TomFischer added some commits Oct 4, 2017

@TomFischer TomFischer force-pushed the TomFischer:NodalSourceTerms branch from c4018dc to 1d621ca Nov 8, 2017

@TomFischer TomFischer merged commit 39a862e into ufz:master Nov 9, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@TomFischer TomFischer deleted the TomFischer:NodalSourceTerms branch Nov 9, 2017

bilke added a commit to bilke/ogs that referenced this pull request Nov 9, 2017

}
void NodalSourceTerm::integrateNodalSourceTerm(
const double t,

This comment has been minimized.

@wenqing

wenqing Nov 9, 2017

Member

@TomFischer t is not used. Would you please send a PR to remove this compilation warning?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment