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

Updated rod-rod, rod-cylinder, self-contact classes, examples, and tests #291

Merged
merged 19 commits into from
Aug 21, 2023

Conversation

Ali-7800
Copy link
Collaborator

Second contact module PR mentioned in Issue
This PR includes:

  • Fixed some strings in test_connection.py and test_contact.py which made the tests pass whether the string was identical or not.
  • Refactor: _check_order -> _check_order_type
  • Added RodRodContact, RodCylinderContact, and RodSelfContact, which are based on ExternalContact and SelfContact from joint.
  • Updated the the rod contact and the rigid body contact examples to use the RodRodContact, RodCylinderContact, and RodSelfContact instead of ExternalContact and RodSelfContact
  • Added test_contact_call_on_systems() in test_contact.py.
  • Updated the test_contact_specific_functions.py, test_contact_common_functions.py, and test_contact_wrapper_classes.py to test RodRodContact, RodCylinderContact, and RodSelfContact.
  • Added tests for testing new contact wrapper classes _check_order_and_type method.

@Ali-7800 Ali-7800 added enhancement New feature or request prio:high Priority level: high labels Aug 10, 2023
@Ali-7800 Ali-7800 added this to the Version 0.3.2 milestone Aug 10, 2023
@Ali-7800 Ali-7800 self-assigned this Aug 10, 2023
@armantekinalp
Copy link
Contributor

@Ali-7800 can you also share the plots or videos for the all examples you have changed in this PR as future reference. Thanks.

@Ali-7800
Copy link
Collaborator Author

Plectonemes Case:

2D_xy_plectonemes.mp4
2D_xz_plectonemes.mp4
2D_zy_plectonemes.mp4
3D_plectonemes.mp4

link_twist_writhe

Solenoids Case:

2D_xy_solenoid.mp4
2D_xz_solenoid.mp4
2D_zy_solenoid.mp4
3D_solenoid.mp4

link_twist_writhe

rod_rod_contact_inclined_validation:

2D_xy_inclined_rods_contact.mp4
2D_xz_inclined_rods_contact.mp4
2D_zy_inclined_rods_contact.mp4
3D_inclined_rods_contact.mp4

inclined_rods_velocity

rod_rod_contact_parallel_validation:

2D_xy_parallel_rods_contact.mp4
2D_xz_parallel_rods_contact.mp4
2D_zy_parallel_rods_contact.mp4
3D_parallel_rods_contact.mp4

parallel_rods_velocity

Rod Cylinder Parallel Contact:

2D_xy_rod_cylinder_contact.mp4
2D_xz_rod_cylinder_contact.mp4
2D_zy_rod_cylinder_contact.mp4
3D_rod_cylinder_contact.mp4

rod_rigid_velocity

Rod Cylinder Inclined Contact:

2D_xy_rod_cylinder_contact.mp4
2D_xz_rod_cylinder_contact.mp4
2D_zy_rod_cylinder_contact.mp4
3D_rod_cylinder_contact.mp4

rod_rigid_velocity

Rod Cylinder Contact with y normal:

2D_cylinder_rod_collision.mp4
cylinder_rod_collision.mp4

rod cylinder with y normal

Rod Cylinder Contact validation:

2D_cylinder_rod_collision.mp4
cylinder_rod_collision.mp4

rod cylinder contact validation

Rod Cylinder Contact Friction:

2D_xy_rod_cylinder_contact.mp4
2D_xz_rod_cylinder_contact.mp4
2D_zy_rod_cylinder_contact.mp4
3D_rod_cylinder_contact.mp4

rod_rigid_velocity

Rod Cylinder Contact Friction Phase Space:

rod_energy_vs_force

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.

Some changes.

elastica/contact_forces.py Outdated Show resolved Hide resolved
elastica/contact_forces.py Outdated Show resolved Hide resolved
tests/test_contact_wrapper_classes.py Outdated Show resolved Hide resolved
tests/test_contact_wrapper_classes.py Outdated Show resolved Hide resolved
tests/test_modules/test_contact.py Show resolved Hide resolved
tests/test_contact_wrapper_classes.py Show resolved Hide resolved
tests/test_contact_specific_functions.py Outdated Show resolved Hide resolved
Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2023

Codecov Report

Patch coverage: 84.69% and project coverage change: -0.71% ⚠️

Comparison is base (fd62afa) 93.71% compared to head (71a4ce3) 93.01%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@               Coverage Diff                @@
##           update-0.3.2     #291      +/-   ##
================================================
- Coverage         93.71%   93.01%   -0.71%     
================================================
  Files                48       49       +1     
  Lines              3055     3334     +279     
  Branches            354      392      +38     
================================================
+ Hits               2863     3101     +238     
- Misses              155      181      +26     
- Partials             37       52      +15     
Files Changed Coverage Δ
elastica/contact_forces.py 81.96% <81.81%> (-5.54%) ⬇️
elastica/contact_utils.py 89.21% <89.21%> (ø)
elastica/joint.py 85.85% <100.00%> (+0.09%) ⬆️
elastica/modules/contact.py 100.00% <100.00%> (+4.76%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Ali-7800 and others added 2 commits August 10, 2023 17:33
Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>
Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>
@armantekinalp
Copy link
Contributor

@Ali-7800 can you also check why coverage is going down by 6%?

@Ali-7800
Copy link
Collaborator Author

@Ali-7800 can you also check why coverage is going down by 6%?

it's because I changed the contact tests from using ExternalContact and SelfContact in joint to the new contact classes. so ExternalContact and SelfContact in joint are not being tested anymore.

@Ali-7800
Copy link
Collaborator Author

@Ali-7800 can you also check why coverage is going down by 6%?

it's because I changed the contact tests from using ExternalContact and SelfContact in joint to the new contact classes. so ExternalContact and SelfContact in joint are not being tested anymore.

Added old tests back to in test_contact_specfic_functions.py, test_contact_common_functions.py, and test_contact_wrapper_classes.py so coverage does not go down. Should be removed later with ExternalContact and SelfContact in joint.py

@Ali-7800
Copy link
Collaborator Author

Added warning message to ExternalContact and SelfContact in joint.py about deprecation.

@Ali-7800 Ali-7800 requested a review from skim0119 August 17, 2023 23:54
@Ali-7800 Ali-7800 requested a review from armantekinalp August 17, 2023 23:54
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.

Minor changes.

elastica/joint.py Outdated Show resolved Hide resolved
elastica/joint.py Outdated Show resolved Hide resolved
tests/test_contact_utils.py Outdated Show resolved Hide resolved
@Ali-7800 Ali-7800 requested a review from armantekinalp August 20, 2023 07:32
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 two minor suggestions

elastica/joint.py Outdated Show resolved Hide resolved
elastica/joint.py Outdated Show resolved Hide resolved
Ali-7800 and others added 2 commits August 20, 2023 11:09
Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>
Co-authored-by: Arman Tekinalp <53585636+armantekinalp@users.noreply.github.com>
Copy link
Collaborator

@bhosale2 bhosale2 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@skim0119 skim0119 left a comment

Choose a reason for hiding this comment

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

Minor changes. Otherwise, LGTM

elastica/joint.py Outdated Show resolved Hide resolved
elastica/joint.py Outdated Show resolved Hide resolved
Ali-7800 and others added 2 commits August 21, 2023 16:58
Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>
Co-authored-by: Seung Hyun Kim <skim0119@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request prio:high Priority level: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants