Xiangyu-Hu / SPHinXsys

SPHinXsys provides C++ APIs for engineering simulation and optimization. It aims at complex systems driven by fluid, structure, multi-body dynamics and beyond. The multi-physics library is based on a unique and unified computational framework by which strong coupling has been achieved for all involved physics.
https://www.sphinxsys.org/
Apache License 2.0
259 stars 199 forks source link

revise shell contact constructor #202

Closed Xiangyu-Hu closed 1 year ago

Xiangyu-Hu commented 1 year ago

Hi, Dong, please check my revision on the constructor and compared the the differences. Xiangyu

DongWuTUM commented 1 year ago

Prof. Hu,

The simplified constructor is well orginazed and more efficient. I should learn form you.

I found a bug (caused by me) when calculating contact_max_. I have corrected it and add regression test for case of 3d_elasticSolid_shell_collision. After corrected bug, the heuristic_limiter can be set as 1.0, i.e, this limiter is not needed anymore. But I still keet it and set it as 0.4 as it do improve the stability.

Dong

DongWuTUM commented 1 year ago

@BenceVirtonomy @FabienPean-Virtonomy Hi, Bence and Fabien, I found a bug when calculating contactmax. After that, the heuristic_limiter can be set as 1.0, i.e, this limiter is not needed anymore. But I still keet it and set it as 0.4 as it do improve the stability. Sorry for misleading. Dong

BenceVirtonomy commented 1 year ago
BenceVirtonomy commented 1 year ago

@BenceVirtonomy @FabienPean-Virtonomy Hi, Bence and Fabien, I found a bug when calculating contactmax. After that, the heuristic_limiter can be set as 1.0, i.e, this limiter is not needed anymore. But I still keet it and set it as 0.4 as it do improve the stability. Sorry for misleading. Dong

Does that mean that it should be calibrated on a test case basis? Meaning different scenarios might require a different constant?

DongWuTUM commented 1 year ago

@BenceVirtonomy @FabienPean-Virtonomy Hi, Bence and Fabien, I found a bug when calculating contactmax. After that, the heuristic_limiter can be set as 1.0, i.e, this limiter is not needed anymore. But I still keet it and set it as 0.4 as it do improve the stability. Sorry for misleading. Dong

Does that mean that it should be calibrated on a test case basis? Meaning different scenarios might require a different constant?

No, it should remain constant for all cases.

DongWuTUM commented 1 year ago

I will try to generate regression test data in Linux system.

Xiangyu-Hu commented 1 year ago

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

DongWuTUM commented 1 year ago

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

Yes, I have.

Xiangyu-Hu commented 1 year ago

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

Yes, I have.

Why did you cancel the CI in linux, as it looks ok?

DongWuTUM commented 1 year ago

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

Yes, I have.

Why did you cancel the CI in linux, as it looks ok?

I think the Linux CI is cancelled by Github automatically, as it has run for more than 360 min.

Xiangyu-Hu commented 1 year ago

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

Yes, I have.

Why did you cancel the CI in linux, as it looks ok?

I think the Linux CI is cancelled by Github automatically, as it has run for more than 360 min.

I will try to generate regression test data in Linux system.

Have you run the test on linux system?

Yes, I have.

Why did you cancel the CI in linux, as it looks ok?

I think the Linux CI is cancelled by Github automatically, as it has run for more than 360 min.

Which suggests that the two-phase dambreak case runs into a dead loop?

DongWuTUM commented 1 year ago

It looks like a dead loop.

Xiangyu-Hu commented 1 year ago

It looks like a dead loop. It passed now. So you can approve the pull request.