Address #148 by adding `ConfigurableFixedConstraint` boundary condition class #143

mstoelzle · 2022-07-18T11:41:46Z

Motivation

Currently, there exists only the option to either have a) no boundary conditions or b) fully fixed boundary conditions. The only functionality in between is that for example translations are allowed, but rotations are constrained and vice-versa. However in practice, there are many situations where we would like to allow certain translations (for example along a plane) or rotations (for example allow yawing of the end of the rod), but constrain all other DoF. The user of the library would have to implement this functionality all themselves inheriting from the FreeBC class. In many cases, such an implementation is not straight-forward. Thus, the library should provide the user the option to configure the BC themselves in a simple fashion. Although it obviously will never cover all the edge-cases a specific project might require, it will give the user an idea / hint / guidance on how to implement a similar solution.

Provided functionality

This PR add the following functionality through the ConfigurableFixedConstraint class:

Using boolean arrays translational_constraint_selector and rotational_constraint_selector, the user can specify which Degrees of Freedom to allow / constrain at the specified nodes.
These boolean arrays have to be specified in the inertial frame.
The ConfigurableFixedConstraint will now only re-set the positions, velocities, directors and omegas to the saved, fixed values for the constrained dimensions. The other DoF are allowed to freely move / rotate.
We can simplify the codebase by now having FixedConstraint be a child class of ConfigurableFixedConstraint. FixedConstraint will still constrain all dimension, e.g. translational_constraint_selector=np.array([True, True, True]) and rotational_constraint_selector=np.array([True, True, True])

Examples for usage

Below examples motivate the usage of this new class.

# How to fix all translational and rotational DoF except allowing twisting around z-axis in inertial frame:

simulator.constrain(rod).using(
    ConfigurableFixedConstraint,
    constrained_position_idx=(0,),
    constrained_director_idx=(0,),
    translational_constraint_selector=np.array([True, True, True]),
    rotational_constraint_selector=np.array([True, True, False]),
)

# How to allow the end of the rod to move in the x-y plane and allow all rotational DoF:

simulator.constrain(rod).using(
    ConfigurableFixedConstraint,
    constrained_position_idx=(-1,),
    translational_constraint_selector=np.array([True, True, False]),
)

I also added an example in examples/BoundaryConditionCases/configurable_fixed_constraint.py, which applies torsion to end tip of a rod. With a fully FixedConstraint the resulting orientation of the first node of the rod would look like this:

Fully Fixed BC:

When we now use rotational_constraint_selector=np.array([True, True, False]) (e.g. allow for yawing / rotation around the z-axis), the orientation of the first node of the rod looks like this:

Free Yawing:

TODO's

Tests for ConfigurableFixedConstraint class
Numba-compatible implementation of nb_constraint_rotational_values
Rotate omega_collection into the inertial frame before applying zero-speed constraints

Request for ideas / feedback

I would like to ask the maintainers specifically for feedback on the current prototype of nb_constraint_rotational_values. Obviously, this is currently not numba-compatible. But more importantly, I would like to ask for your thoughts if constraining the Euler angles is a suitable way to go and if you have other ideas to allow the rotation around certain axis and constrain the rotation around other axis?

…aint`

…figurable-custom-constraint

mstoelzle · 2022-07-19T13:10:48Z

The velocity_collection (of each node) is given in the inertial frame and the omega_collection (of each element) in the local frame, right?

armantekinalp · 2022-07-19T16:41:36Z

The velocity_collection (of each node) is given in the inertial frame and the omega_collection (of each element) in the local frame, right?

Yes @mstoelzle that is correct.

mstoelzle · 2022-07-19T18:50:54Z

The velocity_collection (of each node) is given in the inertial frame and the omega_collection (of each element) in the local frame, right?

Yes @mstoelzle that is correct.

@armantekinalp Thanks for confirming this so quickly!

@armantekinalp @bhosale2 @skim0119 Could you guys have a look at the nb_constraint_rotational_values function and the related questions / requests for feedback in the first post of this PR? Are you happy with the current implemented method and / or do you have ideas for alternative approaches to achieve this functionality?

bhosale2 · 2022-07-19T19:26:19Z

@mstoelzle a couple of comments to start with from my side:

Please usually open an issue first for the problem/feature request where we can discuss things, and then after discussion open a PR accordingly.
Instead of ConfigurableFixedConstraint maybe a CustomConstraintBC would be a shorter to-the-point name? (if I understand correctly what you are trying to do)
I agree that this can be a useful feature to have, but based on my experience of working with pyelastica (and @armantekinalp can concur) enforcing hard boundary conditions (pinning velocities or omegas dynamically), is usually a bit "hard" on the numerical stability of the solver compared to soft ones (say adding a spring-damper force between a constrained node and a point BC moving in space and time). If it works for your problem maybe that could be a useful feature to have in pyelastica forcing? This point is open for discussion.
I would recommend splitting the discussion you have above first into an issue describing the problem (as mentioned in 1.) and then the suggested solution workings here in the PR.

bhosale2

Nice addition @mstoelzle, can you also add tests for the class you add here in tests?

elastica/boundary_conditions.py

…ode review Co-authored-by: Yashraj Bhosale <yashraj.iitb@gmail.com>

codecov-commenter · 2022-07-21T07:13:08Z

Codecov Report

Merging #143 (4682601) into update-0.3.0 (011a0fe) will increase coverage by 0.03%.
The diff coverage is 90.47%.

@@               Coverage Diff                @@
##           update-0.3.0     #143      +/-   ##
================================================
+ Coverage         87.58%   87.62%   +0.03%     
================================================
  Files                43       43              
  Lines              2780     2820      +40     
  Branches            361      368       +7     
================================================
+ Hits               2435     2471      +36     
- Misses              326      328       +2     
- Partials             19       21       +2

Impacted Files	Coverage Δ
elastica/boundary_conditions.py	`95.00% <90.47%> (-1.43%)`	⬇️

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 011a0fe...4682601. Read the comment docs.

- Application of end-forces to test fixed translational constraint and fixed x- and y- rotations - Application of torsional torque to test free yawing of BC

bhosale2

LGTM, @mstoelzle one last thing–can you restore back the .lock file from the update-0.3.0 branch? since no new dependencies were added (no change to .toml)

mstoelzle · 2022-07-22T16:43:22Z

LGTM, @mstoelzle one last thing–can you restore back the .lock file from the update-0.3.0 branch? since no new dependencies were added (no change to .toml)

Thanks! Yes, sorry - I just reverted the poetry.lock file. Didn't mean to commit it to the branch...

armantekinalp

@mstoelzle I asked some minor changes. One thing is could you please also update example ReadMe and add boundary condition examples.

elastica/boundary_conditions.py

tests/test_boundary_conditions.py

elastica/boundary_conditions.py

Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>

- Legacy implementation is most likely faster / more efficient for the simple case of constraining all DoF

mstoelzle · 2022-07-23T08:29:22Z

@mstoelzle I asked some minor changes. One thing is could you please also update example ReadMe and add boundary condition examples.

I updated the README in bdec395

armantekinalp

LGTM, nice work!

skim0119

Please add GeneralConstraint in docs/api/constraints.rst

elastica/boundary_conditions.py

Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>

…ational_constraint_selector` - Declare `translational_constraint_selector` and `rotational_constraint_selector` as optional in docstring - Add `translational_constraint_selector` and `rotational_constraint_selector` as params to `__init__` instead of using `kwargs`

mstoelzle · 2022-07-24T20:27:30Z

Please add GeneralConstraint in docs/api/constraints.rst

I added the GeneralConstraint to the constraints.rst in 2cdc2f9

skim0119 · 2022-07-24T20:29:40Z

Please add GeneralConstraint in docs/api/constraints.rst

I added the GeneralConstraint to the constraints.rst in 2cdc2f9

Please add them on the table above as well.

https://github.com/GazzolaLab/PyElastica/blob/master/docs/api/constraints.rst#description
https://github.com/GazzolaLab/PyElastica/blob/master/docs/api/constraints.rst#compatibility

mstoelzle · 2022-07-24T20:34:01Z

Please add GeneralConstraint in docs/api/constraints.rst

I added the GeneralConstraint to the constraints.rst in 2cdc2f9

Please add them on the table above as well.

https://github.com/GazzolaLab/PyElastica/blob/master/docs/api/constraints.rst#description https://github.com/GazzolaLab/PyElastica/blob/master/docs/api/constraints.rst#compatibility

Sorry - didn't see that. Done in 4682601

skim0119

LGTM. Good Work!

mstoelzle added 6 commits July 15, 2022 22:53

Start implementing ConfigurableFixedConstraint

1836ba4

Replace deprecated np.bool with python bool

aaca3f9

First working, preliminary implementation of `ConfigurableFixedConstr…

2ccef47

…aint`

Add BC configurable_fixed_constraint example

1c8551a

Run make black

c6c1018

Add more code comments to nb_constraint_rotational_values

ba494fd

mstoelzle requested review from armantekinalp, bhosale2 and skim0119 as code owners July 18, 2022 11:41

mstoelzle added 4 commits July 18, 2022 13:42

Fix bug in docstring example

5b6a1b4

Fix bug where allowed directors are not written to director collection

448fcc1

Merge remote-tracking branch 'upstream/update-0.3.0' into feature/con…

2e7bf34

…figurable-custom-constraint

Some reformatting using poetry black

14aaffe

Rotate angular velocities to inertial frame before applying constraint

bffd3cc

bhosale2 assigned mstoelzle Jul 19, 2022

bhosale2 added enhancement New feature or request discussion Topic that needs to be discussed. labels Jul 19, 2022

bhosale2 added this to the Version 0.3 milestone Jul 19, 2022

mstoelzle mentioned this pull request Jul 19, 2022

Inconsistent docstring in FixedConstraint class #142

Closed

mstoelzle changed the title ~~[WIP] Add ConfigurableFixedConstraint boundary condition class~~ [WIP] Address #148 by adding ConfigurableFixedConstraint boundary condition class Jul 19, 2022

bhosale2 linked an issue Jul 19, 2022 that may be closed by this pull request

Add ConfigurableFixedConstraint boundary condition class #148

Closed

bhosale2 requested changes Jul 20, 2022

View reviewed changes

armantekinalp requested changes Jul 20, 2022

View reviewed changes

elastica/boundary_conditions.py Outdated Show resolved Hide resolved

elastica/boundary_conditions.py Outdated Show resolved Hide resolved

Some minor suggestions to improve docstrings and code comments from c…

5bfdfff

…ode review Co-authored-by: Yashraj Bhosale <yashraj.iitb@gmail.com>

Merge branch 'update-0.3.0' into feature/configurable-custom-constraint

bd15e96

mstoelzle added 3 commits July 22, 2022 13:30

Adapt setting of ConfigurableConstraint example

8d91580

- Application of end-forces to test fixed translational constraint and fixed x- and y- rotations - Application of torsional torque to test free yawing of BC

Some formatting

4f2e7a2

Set simulation duration to 10s

86d016c

bhosale2 approved these changes Jul 22, 2022

View reviewed changes

Revert lock file to 0.3.0 branch

ef7954f

armantekinalp requested changes Jul 22, 2022

View reviewed changes

mstoelzle and others added 7 commits July 23, 2022 09:56

Remove _inv_rotate import from boundary_conditions.py

4d53264

Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>

Use batch_matrix_transpose instead of np.transpose

44bb1b4

Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>

Fix syntax errors of previous commits

51f3a91

Rename ConfigurableConstraint to GeneralConstraint

4c06b0e

Merge branch 'update-0.3.0' into feature/configurable-custom-constraint

7020480

Restore logic of legacy FixedConstraint implementation

b8eca47

- Legacy implementation is most likely faster / more efficient for the simple case of constraining all DoF

Add README entry for BoundaryConditionsCases

bdec395

armantekinalp approved these changes Jul 23, 2022

View reviewed changes

skim0119 requested changes Jul 24, 2022

View reviewed changes

mstoelzle and others added 5 commits July 24, 2022 22:09

Merge branch 'update-0.3.0' into feature/configurable-custom-constraint

eb98cd7

Implement some docstring and typing suggestions

a68f2e2

Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>

Fix some bugs in previous commit

5325532

Add GeneralConstraint to docs/api/constraints.rst

2cdc2f9

Add GeneralConstraint also to compatibility and description section

4682601

skim0119 requested changes Jul 25, 2022

View reviewed changes

skim0119 approved these changes Jul 25, 2022

View reviewed changes

skim0119 merged commit 129c35e into GazzolaLab:update-0.3.0 Jul 25, 2022

bhosale2 mentioned this pull request Jul 25, 2022

Add ConfigurableFixedConstraint boundary condition class #148

Closed

Address #148 by adding ConfigurableFixedConstraint boundary condition class #143

Address #148 by adding ConfigurableFixedConstraint boundary condition class #143

Uh oh!

Conversation

mstoelzle commented Jul 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Provided functionality

Examples for usage

TODO's

Request for ideas / feedback

Uh oh!

mstoelzle commented Jul 19, 2022

Uh oh!

armantekinalp commented Jul 19, 2022

Uh oh!

mstoelzle commented Jul 19, 2022

Uh oh!

bhosale2 commented Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bhosale2 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jul 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bhosale2 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstoelzle commented Jul 22, 2022

Uh oh!

armantekinalp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mstoelzle commented Jul 23, 2022

Uh oh!

armantekinalp left a comment

Choose a reason for hiding this comment

Uh oh!

skim0119 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mstoelzle commented Jul 24, 2022

Uh oh!

skim0119 commented Jul 24, 2022

Uh oh!

mstoelzle commented Jul 24, 2022

Address #148 by adding `ConfigurableFixedConstraint` boundary condition class #143

Address #148 by adding `ConfigurableFixedConstraint` boundary condition class #143

mstoelzle commented Jul 18, 2022 •

edited

Loading

bhosale2 commented Jul 19, 2022 •

edited

Loading

codecov-commenter commented Jul 21, 2022 •

edited

Loading

bhosale2 left a comment •

edited

Loading

armantekinalp left a comment •

edited

Loading