Pyelastica API changes

[skim0119]

Issue by armantekinalp
Monday Apr 13, 2020 at 16:07 GMT
Originally opened as https://github.com/GazzolaLab/elastica-python/issues/73

---

These comments are from @tp5uiuc regarding the API of Elastica. I think they are good comments and we should try to accomplish these changes. @tp5uiuc and @nmnaughton please go over the below list and add your comments. Then after we finalize our discussion, I will start making changes.

• Refactor wrappers to modules #168
• Changing OneEndFixedRod to OneEndFixedBC . ([Dev Note] Boundary condition issues. #55)
  • Forcing/Constraint/Connection: rod ->system if it supports rigid-body #174
• Changing callback functions to observer functions
• Changing CallBackBaseClass to AbstractObserver or ObserverBaser.
  • @skim449: I'm scratching this out, because I think Base class is a somewhat common trend in many Python packages I'm seeing. There is really no community standard for this other than OOP languages. The word abstract could be too restrictive.
• Reducing import statements, we can do
  -- from elastica.timesteppers import integrate, PositionVerlet
  -- from elastica.wrappers import Connections, Constraints
  -- from elastica import *

Other Refactor Request

-- Dissipation constant name to include mass

• ExponentialDamping -> AnalyticalLinearDamping refac: exp damper --> analytical linear damper #163

[skim0119]

Comment by skim0119
Monday Dec 27, 2021 at 02:16 GMT

---

If we are going to refactor these, we should do it as soon as possible. To have a smooth transition, we can keep soft-reference link and have deprecated warning. In the version after, we cut-off the reference and throw deprecated error.

• I think callback name is okay, but maybe it is not common to have one callback for each system(rod).
• I think the word 'Base' is also fine. I'm seeing many ML python libraries use this convention. Only problem is that it doesn't force users to override the method. If this is needed, we should use abstract from abc.
• Reducing import statement is good idea, but we should probably make it clear where each modules are coming from, especially in tutorial. I think from elastica import * makes everything very confusing. We should consider using import elastica as el or something.

[nmnaughton]

I am looking to address the BC issue in #54. Some other thoughts:

• I agree that there is a better option than wrappers. I don't know if addons or features are the best terminologies though. Maybe 'functionalities'? (I don't really like that either though). tbd...
• I like changing callback to something else too. What about logging? For non-programmers, that might be a bit more intuitive. Also, we could use 'Default' instead of 'Base' to address @skim0119's point about the overriding issue.
• I also agree about the import statement issue, there should be a cleaner approach. import elastica as *** seems more consistent with norms. el works, but maybe there is a better contraction?

[skim0119]

• Since we are using the composition concept to model the simulator, Wrapper to Component is also a good candidate. To be explicit, we can rename the directory simulator and use the work component in the code and descriptions.
• I think callback is a pretty common word for a functionality that is a little different than logging. I think logging should be one type of callback (although it is only type we have implemented 😆 ). For example, we might want to have a callback that explicitly checks for nan in each step.
• We can start using import elastica as *** as a new convention when writing documentation. Nothing is set on stone, but in elastica++ I use

namespace efo = ::elastica::forcing;
namespace eco = ::elastica::constraints;
namespace econ = ::elastica::connections;

[nmnaughton]

• I like Simulator and Component! The idea that you need to add the needed components to your simulator should be pretty intuitive to users.
• Good point about the callback concept. I do think maybe a default 'logging' callback could be useful for users who just want to get that functionality quickly, but you are right, we should allow those other extensions without limiting things. Maybe have CallBackBaseClass and also provide a DefaultLogger that is derived from it?

[skim0119]

I refactored the class OneEndFixedRod to OneEndFixedBC in wip_constraint branch. I left a deprecation warning message and alias. The alias will be removed in v0.3.0

[bhosale2]

Closed since completed.