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

fluid shell interaction model. #184

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Conversation

DrChiZhang
Copy link
Collaborator

1, add fluid shell interaction dynamics.
2, add new test case of sloshing flow with baffle
3, generalise complex dynamics according with adding third contact body to contact body list.

This reverts commit 65bc702, reversing
changes made to 125be31.
Here, cmake is modified, pay attention when PR.
Real mass_i = mass_[index_i];
VariableType &variable_i = variable_[index_i];

std::array<Real, MaximumNeighborhoodSize> parameter_b;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These arrays can cause memory errors for multiresolution, where the neighborhood size is larger than the MaximumNeighborhoodSize variable. I think we mentioned this once but I don't remember what our conclusion was with prof. Hu. It's not very efficient either, perhaps @FabienPean-Virtonomy has a better solution?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it happened I had some problems there in the past, it was more related to some system resolution compared to particle spacing. There will be issues for multiresolution definitely.

Two simpler options I see:

  • The simpler fix would be using a std::vector with reserved memory initial memory (by calculating max neighborhood size)
  • Using a couple array + vector and adjusting the loops to fill the vector once the array is filled.

Otherwise other solutions could involve a different container outside STL (but require adding dependency), thread local vector (need to be confident about concurrency)

{
size_t index_j = contact_neighborhood.j_[n];

parameter_b[n] = contact_particles_[k]->DegeneratedSpacing(index_j) * eta_ * contact_neighborhood.dW_ijV_j_[n] * Vol_i * dt / contact_neighborhood.r_ij_[n];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is "DegeneratedSpacing"? Could we have a more understandable name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shell formulation is called Degenerated Approach, therefore, DegeneratedSpacing used here denoting the spacing being degenerated.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does degenerated mean in this context? Can you explain it please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I thought is " shell particle is degenerated (having low standard or behavior) ) from volume particle in the thickness spacing. Then, with shell particle is interacting with volume particle, the degenerated spacing is considered.
If you have other suggestion, please let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that you consider the shell particle as a squashed solid particle in the thickness dimension? If yes, I would just call it ThicknessAdjustedSpacing for example, much clearer I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name it "Thinkness" is one of my old choices, while this name is meaningless for volume particles as they do not have thickness. Maybe "ReducedThickness", "ReducedCrossSection", "DegeneratedThickness" or "DegeneratedCrossSection" is more readable.

Copy link
Collaborator

@BenceVirtonomy BenceVirtonomy Dec 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ReducedThickness", "ReducedCrossSection" sound good to me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok my problem was more with spacing than degenerate, I went to look in the code and it is basically the reduced or degenerated dimension (return 1 for volume, thickness for shells, and area for beams).
I would either go by crossSectionDimension or reducedDimension (or their getXXX variant)

Also why is the naming convention changing ? It was using camelCase before, now part of the new code is in PascalCase. Please change uniformize back to camelCase

It is implemented by virtual function, which I am not so comfortable with, but I don't have a straightforward drop-in replacement to offer at the moment.

@BenceVirtonomy
Copy link
Collaborator

@ChiZhangatTUM We would like to make a 3d test here too. I was wondering if you already have the elastic beam in a flow case at least in 2d? I mean test_2d_fsi2 case with the shell? I'm asking you to avoid doing something you have done already.
If you have it, I would try to expand it to 3d otherwise build it from scratch.

@DrChiZhang
Copy link
Collaborator Author

@ChiZhangatTUM We would like to make a 3d test here too. I was wondering if you already have the elastic beam in a flow case at least in 2d? I mean test_2d_fsi2 case with the shell? I'm asking you to avoid doing something you have done already.
If you have it, I would try to expand it to 3d otherwise build it from scratch.

Sure, I have already done some work.
You can follow tests/user-exampels/test-2d-square-fsi in fluid-shell-interaction branch .

@BenceVirtonomy
Copy link
Collaborator

@ChiZhangatTUM We would like to make a 3d test here too. I was wondering if you already have the elastic beam in a flow case at least in 2d? I mean test_2d_fsi2 case with the shell? I'm asking you to avoid doing something you have done already.
If you have it, I would try to expand it to 3d otherwise build it from scratch.

Sure, I have already done some work. You can follow tests/user-exampels/test-2d-square-fsi in fluid-shell-interaction branch .

Ahh, I see. Could you make a draft PR from that branch? That way we can all see the changes there too.

};

/**
* @class DampingPairwiseFromShell
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which test cases uses this class?

variable_[index_i] += parameter_b[n] * (variable_i - variable_k[index_j]) / (mass_i - 2.0 * parameter_b[n]);
}
// backward sweep
for (size_t n = contact_neighborhood.current_size_; n != 0; --n)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do feel that we need not repeat so much codes here.

@@ -36,7 +36,7 @@ namespace SPH
Vecd nablaW_ijV_j = contact_neighborhood.dW_ijV_j_[n] * contact_neighborhood.e_ij_[n];

// acceleration for transport velocity
acceleration_trans -= 2.0 * nablaW_ijV_j;
acceleration_trans -= 2.0 * nablaW_ijV_j * this->contact_particles_[k]->DegeneratedSpacing(index_j);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will decrease efficiency of the original code a lot, as it needs another value from particle j.

@@ -88,7 +88,8 @@ namespace SPH
Neighborhood &contact_neighborhood = (*this->contact_configuration_[k])[index_i];
for (size_t n = 0; n != contact_neighborhood.current_size_; ++n)
{
sigma += contact_neighborhood.W_ij_[n] * contact_inv_rho0_k * contact_mass_k[contact_neighborhood.j_[n]];
Real mass = this->contact_particles_[k]->ParticleMass(contact_neighborhood.j_[n]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue on decreasing efficiency. We should have a discussion on this.

@@ -0,0 +1,136 @@
/* -------------------------------------------------------------------------*
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be long to fluid dynamics.

* @brief Abstract base class for general fluid-shell interaction model.
* @note Here,
*/
template<class BaseIntegrationType>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinkin the current formulation is too complicated. We need a discuss on this.

@Xiangyu-Hu
Copy link
Owner

1, add fluid shell interaction dynamics. 2, add new test case of sloshing flow with baffle 3, generalise complex dynamics according with adding third contact body to contact body list.

Dear Chi, I think that we need a discussion on the pull request to clarify several issues.

@DrChiZhang
Copy link
Collaborator Author

1, add fluid shell interaction dynamics. 2, add new test case of sloshing flow with baffle 3, generalise complex dynamics according with adding third contact body to contact body list.

Dear Chi, I think that we need a discussion on the pull request to clarify several issues.

Sure, I will be office from Tuesday to Thursday.

@DrChiZhang
Copy link
Collaborator Author

Hi all, As we have lots of issues on this PR. I will close it without merging in the following days.

@BenceVirtonomy
Copy link
Collaborator

Hi all, As we have lots of issues on this PR. I will close it without merging in the following days.

You don't need to close it Chi, unless you want to start from scratch. You can set it back to draft so that we can still see the changes.

@DrChiZhang
Copy link
Collaborator Author

I will add it to the user_examples of this current branch in the next commit, and you can modify and add it to test with proper validation.

@BenceVirtonomy

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants