-
Notifications
You must be signed in to change notification settings - Fork 8
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
Cyclic loading test #53
Conversation
shadisharba
commented
Feb 21, 2018
- A slight modification of the cyclic loading implementation.
- Cyclic loading test is added.
- The damage model name is changed from "ChabocheDamage" to "J2PlasticityDamage".
It seems that all new commits appear in the same pull request, sorry for that. |
@@ -37,7 +37,7 @@ | |||
"Mesh" : [{ | |||
"Name" : "beam", | |||
"ConstitutiveModel" : { | |||
"Name" : "ChabocheDamage", | |||
"Name" : "J2PlasticityDamage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What makes more sense here is to keep the overarching yield theory (J2) and append a damage model to that. So more like
"Name" : "J2Plasticity",
"Damage" : "Isotropic",
or
"Damage" : "Chaboche"
That way additional models can be added with ease.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good overall.
src/mesh/generic/Boundary.cpp
Outdated
@@ -109,19 +114,17 @@ void Boundary::generate_sinusoidal(json const& boundary, | |||
|
|||
auto const scale_factor = number_of_cycles[i] * period[i] / time_block.back(); | |||
|
|||
time_block = time_block | view::transform([scale_factor, new_times](double x) { | |||
return x * scale_factor + new_times.back(); | |||
time_block = time_block | view::transform([scale_factor, times](double x) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the last entry should be captured by the lambda otherwise we're taking a copy of the times array when we only want the .back() element right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Capturing by reference solves the problem, right?
src/mesh/generic/Boundary.cpp
Outdated
new_loads = new_times | view::transform([i, amplitude, omega, phase](double x) { | ||
return amplitude[i] * std::sin(omega * x + phase[i]); | ||
}); | ||
loads = times | view::transform([i, amplitude, omega, phase](double x) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps capture by reference for amplitude and phase:
&litude, ..., &phase](
to avoid a copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One day you won't need to tell me about references and const :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small efficiency changes 👍
src/solver/adaptive_time_step.cpp
Outdated
@@ -114,8 +114,17 @@ void adaptive_time_step::parse_input(json const& increment_data, double const ma | |||
// Initial factor determined by input | |||
initial_time = increment_data["Increments"]["Initial"]; | |||
|
|||
minimum_increment = increment_data["Increments"]["Minimum"]; | |||
maximum_increment = increment_data["Increments"]["Maximum"]; | |||
auto adaptive_increment = increment_data["Increments"]["Adaptive"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. The JSON spec allows boolean types, so to avoid a string allocation and compare we can just do:
bool const is_adaptive_increment = increment_data["Increments"]["Adaptive"];
minimum_increment = is_adaptive_increment ? increment_data["Increments"]["Minimum"] : initial_time;
...
Since only two possibilities are likely. The JSON would look like
...
"Adaptive" : false,
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice