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.

Sign up for GitHub

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

Jump to bottom

Dissipation constant fix (#81) #87

Merged
merged 26 commits into from
Jun 1, 2022
Merged

Dissipation constant fix (#81) #87

merged 26 commits into from
Jun 1, 2022

Conversation

bhosale2
Copy link
Collaborator

@bhosale2 bhosale2 commented May 17, 2022 •
edited
Loading

Fixes #81, as per our discussion @armantekinalp this PR is WIP and we will merge once the dissipation constants are rescaled for examples, and all examples with nu have been confirmed to run fine. Other than that, please go through PR and let me know if any changes are needed.

@bhosale2 bhosale2 added bug Something isn't working help wanted Extra attention is needed update Update the codebase labels May 17, 2022
@bhosale2 bhosale2 added this to the Version 0.3 milestone May 17, 2022
@bhosale2 bhosale2 linked an issue May 17, 2022 that may be closed by this pull request
Dissipation constant should be scaled with mass instead of lengths #81
Closed
@skim0119 skim0119 self-requested a review May 17, 2022 05:39
@skim0119 skim0119 changed the base branch from update-0.3.0 to patch_dissipation May 17, 2022 05:40
@armantekinalp
Copy link
Contributor

Can we fast-forward patch_dissipation branch to update-0.2.3 ? I think patch_dissipation is created from the master and this PR shows new commits which are already committed in update-0.2.3

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2022 •
edited
Loading

Codecov Report

Merging #87 (b13fd45) into patch_dissipation (c58e34f) will not change coverage.
The diff coverage is n/a.

❗ Current head b13fd45 differs from pull request most recent head edc2e9a. Consider uploading reports for the commit edc2e9a to get more accurate results

@@           Coverage Diff           @@
##   patch_dissipation   #87   +/-   ##
=======================================
=======================================

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c58e34f...edc2e9a. Read the comment docs.

@armantekinalp
Copy link
Contributor

I ran the cases on cluster, and following ones require tuning:

  • Helical buckling case exploded for larger than 100 elements.
  • Muscular flagella shows a difference with elastica cpp implementation
  • Continuum snake velocity profile is not periodic.
  • Muscular snake case exploded.
  • Rod self contact cases (solenoid and plectoneme) exploded.

@bhosale2
Copy link
Collaborator Author

bhosale2 commented May 21, 2022 •
edited
Loading

@armantekinalp as discussed, we now decide to premultiply both the velocity and damping terms by mass (to include tapering effects), as included in 89b7bc8.

(List below needs to be checked with nu scaled as nu = old_nu / density / base_cs_area)

  • axial stretching
  • binder stuff?
  • continuum flagella
  • continuum snake
  • Experimental cases - parallel connection
  • friction validation cases
  • helical buckling
  • joint cases
  • muscular flagella
  • muscular snake
  • rod-rod contact cases
  • rod self contact cases
  • timoshenko

@bhosale2 bhosale2 changed the base branch from patch_dissipation to update-0.3.0 May 31, 2022 17:41
@bhosale2 bhosale2 changed the base branch from update-0.3.0 to patch_dissipation May 31, 2022 17:42
@bhosale2 bhosale2 changed the base branch from patch_dissipation to update-0.3.0 May 31, 2022 18:56
armantekinalp
armantekinalp reviewed Jun 1, 2022
View reviewed changes
Copy link
Contributor

@armantekinalp armantekinalp left a comment

Choose a reason for hiding this comment

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

LGTM, just two minor changes

elastica/memory_block/memory_block_rod.py Outdated Show resolved Hide resolved
elastica/memory_block/memory_block_rod.py Outdated Show resolved Hide resolved
armantekinalp
armantekinalp approved these changes Jun 1, 2022
View reviewed changes
Copy link
Contributor

@armantekinalp armantekinalp left a comment

Choose a reason for hiding this comment

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

LGTM

@bhosale2 bhosale2 merged commit cbc24d9 into GazzolaLab:update-0.3.0 Jun 1, 2022
@bhosale2 bhosale2 deleted the 81_dissipation_constant_fix branch June 1, 2022 02:59
@bhosale2 bhosale2 mentioned this pull request Jun 1, 2022
Closed
@skim0119 skim0119 changed the title [WIP] (#81) dissipation constant fix Dissipation constant fix (#81) Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@armantekinalp armantekinalp armantekinalp approved these changes

@skim0119 skim0119 Awaiting requested review from skim0119

Assignees

@bhosale2 bhosale2

@armantekinalp armantekinalp

Labels
bug Something isn't working help wanted Extra attention is needed update Update the codebase
Projects
None yet
Milestone
Version 0.3
Development

Successfully merging this pull request may close these issues.

Dissipation constant should be scaled with mass instead of lengths
4 participants
@bhosale2 @armantekinalp @codecov-commenter @skim0119