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

Dirichlet boundary condition within a time interval #2272

Merged
merged 11 commits into from Nov 28, 2018

Conversation

Projects
None yet
3 participants
@wenqing
Copy link
Member

wenqing commented Nov 27, 2018

This PR presents a Dirichlet boundary condition within a time interval.

The benchmark is about a square domain under liquid pressure on its top and bottom. On the top boundary, pressure is always 1.0e+5, while on the bottom boundary, pressure is 1.0e+6 within a time interval of [0, 10] (see the following figure):
tintervalbc

            Left, t<=10.0;                            right, t>10

@wenqing wenqing force-pushed the wenqing:time_dependent_D_BC branch from 1c12b93 to 7fc0b17 Nov 27, 2018

* Class for a time interval, which has a member to check whether the given time
* is in this time interval.
*/
class TimeInterval

This comment has been minimized.

@endJunction

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

added.

{
return (current_time >= _start_time && current_time <= _end_time)
? true
: false;

This comment has been minimized.

@endJunction

endJunction Nov 27, 2018

Member

The result of comparison is already a boolean, I'd drop the true:false

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

changed.

{
}

bool isInThisTimeInterval(const double current_time) const

This comment has been minimized.

@endJunction

endJunction Nov 27, 2018

Member

Maybe isActive, s.t.

TimeInterval time_interval{start, end};
...
if (time_interval.isActive(t)) { ... }

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

changed as if (time_interval.contains(t)).

{
if (_time_interval->isInThisTimeInterval(t))
{
return getEssentialBCValuesLocal(t, x, bc_values);

This comment has been minimized.

@endJunction

endJunction Nov 27, 2018

Member

The return type of the function is void...

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

changed.


const double end_time =
//! \ogs_file_param{prj__process_variables__process_variable__boundary_conditions__boundary_condition__DirichletWithinTimeInterval__end_time}
config.getConfigParameter<double>("end_time");

This comment has been minimized.

@endJunction

endJunction Nov 27, 2018

Member

Just a suggestion: I'd rather extract the config parsing into createTimeInterval() and have the <time_interval> tag in the project file:

...
<time_interval>
    <start>0</start>
    <end>1</end>
</time_interval>

Because parsing of the time interval is not a task for a boundary condition, in my opinion.

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

Added.

*/
#pragma once

namespace NumLib

This comment has been minimized.

@TomFischer

TomFischer Nov 28, 2018

Member

Why is it in NumLib? I would put it in BaseLib.

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

At beginning of my implementation, I had put it in BaseLib. Later on, I thought that this class is related to the time stepping only. Therefore I moved it to NumLib/TimeStepping. If you still think BaseLib is the best choice for it after reading this reply, I will move it BaseLib again.

This comment has been minimized.

@endJunction

endJunction Nov 28, 2018

Member

Could be BaseLib because it's rather general, or ProcessLib/BC/ because this is the only place where it is used now.

This comment has been minimized.

@wenqing

wenqing Nov 28, 2018

Author Member

Since both of you prefer to move to BaseLib, I have moved it to there but kept the path of parameter documentation.

@wenqing

This comment has been minimized.

Copy link
Member Author

wenqing commented Nov 28, 2018

@endJunction Introduced two functions to avoid class inheritance while to avoid source code duplication.

@endJunction
Copy link
Member

endJunction left a comment

Looks good to me.
To move or not to move is up to you.

@wenqing wenqing force-pushed the wenqing:time_dependent_D_BC branch from 9a4459e to deac805 Nov 28, 2018

@wenqing wenqing force-pushed the wenqing:time_dependent_D_BC branch from 035110b to 96ded07 Nov 28, 2018

@endJunction endJunction merged commit d822261 into ufz:master Nov 28, 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
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.