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

Type-hinting elastica #367

Merged
merged 175 commits into from
Jun 28, 2024
Merged

Type-hinting elastica #367

merged 175 commits into from
Jun 28, 2024

Conversation

skim0119
Copy link
Collaborator

@skim0119 skim0119 commented Apr 27, 2024

Type-hinting PyElastica

Resolve #255

Note

  • This PR includes type-hinting all backend files to pass mypy test.
  • The entire purpose of type-hinting python library is to enhance the readability and to reduce ambiguity, not to increase the codebase.

Major Updates

  • mypy test is added as part of github-action (Makefile)
  • protocol trees for different components in PyElastica (described below)
  • Common typing aliases in elastica/typing.py file. (described below)
  • System (RodType, RigidBodyType, etc) used for each feature are now typed properly.
  • In Contact feature: refactor system validation messages. (i.e. common_check_systems_identity, common_check_systems_difference, common_check_systems_validity)
  • In InteractionPlane: this operator was never possible to be used. Now, it is part of forcing feature, and can be used like external forces. (apply_normal_force --> apply_forces)
  • elastica/memory_block/memory_block_rod_base.py --> elastica/memory_block/utils.py.
  • Previously, it was possible to pass system id (integer) to .using statement. It was useful for some testing features, but it is an over-exploitation of dynamic types. Now, we only support passing exact object (RodType, RigidBodyType, etc)
  • During finalize step, the group of rod and rigid body are converted into a single rod and rigid body, and added to __final_systems.
  • Integrator routine is simplified:
timestepper: SymplecticStepperProtocol = PositionVerlet()
dt = 1e-3
for _ in range(100):  # 100 timestepping
  timestepper.step(simulator, time=0.0, dt=dt)
  • Previous extend_stepper_interface and integrate should work same as before.
    • Added deprecation tag on extend_stepper_interface.
    • People should be able to just use integrate or custom stepping as above.

Minor Changes

  • Necessary updates in unittests
  • autoflake8 to autoflake: autoflake8 was made because the original autoflake was out of maintenance. Since autoflake is back on maintenance, autoflake8 is now deprecated. (Makefile)
  • KnotTheoryCompatibleProtocol moved into CosseratRodProtocol.
  • njit decorators are skipped from mypy check: numba implicitly modify the return type, which is hard to trace exactly. For readability, we keep numpy type-hinting syntax.
  • Deprecated functionalities are removed to avoid unnecessary typing:
    • SelfContact
    • ExternalContact

Protocols

Systems

flowchart LR
  direction RL
  subgraph Systems Protocol
    direction RL
    SLBD(SlenderBodyGeometryProtool)
    SymST["SymplecticSystem:\n• KinematicStates/Rates\n• DynamicStates/Rates"]
    style SymST text-align:left
    ExpST["ExplicitSystem:\n• States (Unused)"]
    style ExpST text-align:left
    P((position\nvelocity\nacceleration\n..)) --> SLBD
    subgraph StaticSystemType
        Surface
        Mesh
    end
    subgraph SystemType
        direction TB
        Rod
        RigidBody 
    end
    SLBD --> SymST
    SystemType --> SymST
    SLBD --> ExpST
    SystemType --> ExpST
  end
  subgraph Timestepper Protocol
    direction TB
    StP["StepperProtocol\n• step(SystemCollection, time, dt)"]
    style StP text-align:left
    SymplecticStepperProtocol["SymplecticStepperProtocol\n• PositionVerlet"]
    style SymplecticStepperProtocol text-align:left
    ExpplicitStepperProtocol["ExpplicitStepperProtocol\n(Unused)"]
  end
  
  subgraph SystemCollection
    
  end
  SymST --> SystemCollection --> SymplecticStepperProtocol
  ExpST --> SystemCollection --> ExpplicitStepperProtocol
  StaticSystemType --> SystemCollection
  
Loading

System Collection (Build memory block)

flowchart LR
  Sys((Systems))
  St((Stepper))
  subgraph SystemCollectionType
    direction LR
    StSys["StaticSystem:\n• Surface\n• Mesh"]
    style StSys text-align:left
    DynSys["DynamicSystem:\n• Rod\n  • CosseratRod\n• RigidBody\n  • Sphere\n  • Cylinder"]
    style DynSys text-align:left

    BlDynSys["BlockSystemType:\n• BlockCosseratRod\n• BlockRigidBody"]
    style BlDynSys text-align:left

    F{{"Feature Group (OperatorGroup):\n• Synchronize\n• Constrain values\n• Constrain rates\n• Callback"}}
    style F text-align:left
  end
  Sys --> StSys --> F
  Sys --> DynSys -->|Finalize| BlDynSys --> St
  DynSys --> F <--> St
  
Loading

System Collection (Features)

flowchart LR
  Sys((Systems))
  St((Stepper))
  subgraph SystemCollectionType
    direction LR
    StSys["StaticSystem:\n• Surface\n• Mesh"]
    style StSys text-align:left
    DynSys["DynamicSystem:\n• Rod\n&nbsp;&nbsp;• CosseratRod\n• RigidBody\n&nbsp;&nbsp;• Sphere\n&nbsp;&nbsp;• Cylinder"]
    style DynSys text-align:left
    
    subgraph Feature
    direction LR
    Forcing -->|add_forcing_to| Synchronize
    Constraints -->|constrain| ConstrainValues
    Constraints -->|constrain| ConstrainRates
    Contact -->|detect_contact_between| Synchronize
    Connection -->|connect| Synchronize
    Damping -->|dampen| ConstrainRates
    Callback -->|collect_diagnosis| CallbackGroup
    end
  end
  Sys --> StSys --> Feature
  Sys --> DynSys
  DynSys --> Feature <--> St
  
Loading

Common Types Aliases

included in elastica/typing.py

To use type aliases, developer should explicitly import using from elastica.typing import ... syntax. typing.py is not included in the top access tree (i.e. `elastica/init.py).

  • SystemType : any generic object type for simulator (i.e. Rod, RigidBody, etc.)
  • SystemCollectionType : object collection (i.e. BaseSystemCollection or simulator that user defines)
  • OperatorType : any generic step used in timestepper
  • SteppersOperationstype : collection of OperatorType used to define specific timestepper

Tasks

Useful Links

@skim0119 skim0119 changed the base branch from update-0.3.3 to master April 27, 2024 12:14
@skim0119 skim0119 changed the base branch from master to update-0.3.3 April 27, 2024 12:15
@skim0119 skim0119 self-assigned this Apr 28, 2024
@skim0119 skim0119 added enhancement New feature or request github_actions Pull requests that update GitHub Actions code labels Apr 28, 2024
elastica/_calculus.py Outdated Show resolved Hide resolved
@skim0119 skim0119 requested a review from sy-cui June 26, 2024 13:56
Copy link
Contributor

@sy-cui sy-cui left a comment

Choose a reason for hiding this comment

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

Great work. Minor comments

elastica/boundary_conditions.py Outdated Show resolved Hide resolved
elastica/boundary_conditions.py Outdated Show resolved Hide resolved
elastica/interaction.py Show resolved Hide resolved
elastica/joint.py Show resolved Hide resolved
elastica/modules/base_system.py Show resolved Hide resolved
elastica/systems/analytical.py Outdated Show resolved Hide resolved
elastica/systems/memory.py Show resolved Hide resolved
elastica/systems/memory.py Show resolved Hide resolved
elastica/rod/factory_function.py Show resolved Hide resolved
elastica/modules/contact.py Show resolved Hide resolved
Copy link

codecov bot commented Jun 27, 2024

Codecov Report

Attention: Patch coverage is 95.47206% with 47 lines in your changes missing coverage. Please review.

Project coverage is 93.96%. Comparing base (5112e05) to head (92d8b91).
Report is 77 commits behind head on update-0.3.3.

Files Patch % Lines
elastica/timestepper/explicit_steppers.py 77.94% 15 Missing ⚠️
elastica/modules/base_system.py 91.93% 2 Missing and 3 partials ⚠️
elastica/interaction.py 82.60% 4 Missing ⚠️
elastica/modules/constraints.py 91.17% 3 Missing ⚠️
elastica/modules/connections.py 94.59% 1 Missing and 1 partial ⚠️
elastica/modules/damping.py 90.90% 2 Missing ⚠️
elastica/modules/memory_block.py 85.71% 1 Missing and 1 partial ⚠️
elastica/rigidbody/rigid_body.py 93.54% 2 Missing ⚠️
elastica/rod/data_structures.py 88.23% 2 Missing ⚠️
elastica/surface/surface_base.py 50.00% 2 Missing ⚠️
... and 7 more
Additional details and impacted files
@@               Coverage Diff                @@
##           update-0.3.3     #367      +/-   ##
================================================
- Coverage         94.00%   93.96%   -0.05%     
================================================
  Files                51       53       +2     
  Lines              3170     3295     +125     
  Branches            347      335      -12     
================================================
+ Hits               2980     3096     +116     
- Misses              140      152      +12     
+ Partials             50       47       -3     

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

Copy link
Contributor

@sy-cui sy-cui 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

@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

@Ali-7800 Ali-7800 left a comment

Choose a reason for hiding this comment

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

LGTM!

@skim0119 skim0119 merged commit a7ce95c into update-0.3.3 Jun 28, 2024
12 of 13 checks passed
@skim0119 skim0119 deleted the update/mypy branch June 28, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request github_actions Pull requests that update GitHub Actions code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants