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

[Core] Proposition to reduce the number of alias declarations #4788

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

alxbilger
Copy link
Contributor

@alxbilger alxbilger commented Jun 24, 2024

The goal is to reduce the number of alias declarations. I propose 2 methods:

  1. using traits (for example DataVecCoord_t<Out>). This is illustrated in Multi2Mapping.
  2. using a set of macros. The aliases are still declared, but it is hidden inside the macro.

I find the trait approach elegant (nothing is hidden and no use of macro or additional code), but too much verbose when only one template parameter is available (e.g. TetrahedronFEMForceField).

The advantage of the macro is that it does not require changes in the code other than the alias declarations.


By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).


Reviewers will merge this pull-request only if

  • it builds with SUCCESS for all platforms on the CI.
  • it does not generate new warnings.
  • it does not generate new unit test failures.
  • it does not generate new scene test failures.
  • it does not break API compatibility.
  • it is more than 1 week old (or has fast-merge label).
@alxbilger alxbilger added pr: status to review To notify reviewers to review this pull-request pr: clean Cleaning the code labels Jun 24, 2024
@alxbilger
Copy link
Contributor Author

[ci-build][with-all-tests]

@alxbilger alxbilger force-pushed the aliasdeclaration branch 3 times, most recently from 6ecf25b to 1f696af Compare July 3, 2024 07:53
typedef sofa::type::vector<MaterialStiffness> VecMaterialStiffness; ///< a vector of material stiffness matrices
typedef type::Mat<6, 3, Real> StrainDisplacement; ///< the strain-displacement matrix (the transpose, actually)
typedef type::Mat<6, 3, Real_t<DataTypes>> StrainDisplacement; ///< the strain-displacement matrix (the transpose, actually)
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixing real precision in our codebase has the only consequence of making it very harde to track the preservation of a given level of precision (i.e, there is no undesired/un-expected loss).
In previously merged PR related to refactoring/removal of alias declaration the idea was to simply use SReal. So we are compiling SOFA either in double or in float and not a giant mic-mac.

Comment on lines +43 to +45
, d_poisson(initData(&d_poisson, Real_t<DataTypes>(0.3), "poissonRatio", "Poisson ratio in Hooke's law"))
, d_young(initData(&d_young, Real_t<DataTypes>(1000.), "youngModulus", "Young modulus in Hooke's law"))
, d_thickness(initData(&d_thickness, Real_t<DataTypes>(1.), "thickness", "Thickness of the elements"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
, d_poisson(initData(&d_poisson, Real_t<DataTypes>(0.3), "poissonRatio", "Poisson ratio in Hooke's law"))
, d_young(initData(&d_young, Real_t<DataTypes>(1000.), "youngModulus", "Young modulus in Hooke's law"))
, d_thickness(initData(&d_thickness, Real_t<DataTypes>(1.), "thickness", "Thickness of the elements"))
, d_poisson(initData(&d_poisson, SReal(0.3), "poissonRatio", "Poisson ratio in Hooke's law"))
, d_young(initData(&d_young, SReal(1000.), "youngModulus", "Young modulus in Hooke's law"))
, d_thickness(initData(&d_thickness, SReal(1.), "thickness", "Thickness of the elements"))
Comment on lines +130 to +134
Data<Real_t<DataTypes>> d_poisson; ///< Poisson ratio in Hooke's law
Data<Real_t<DataTypes>> d_young; ///< Young modulus in Hooke's law
Data<Real_t<DataTypes>> d_thickness; ///< Thickness of the elements
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Data<Real_t<DataTypes>> d_poisson; ///< Poisson ratio in Hooke's law
Data<Real_t<DataTypes>> d_young; ///< Young modulus in Hooke's law
Data<Real_t<DataTypes>> d_thickness; ///< Thickness of the elements
Data<SReal> d_poisson; ///< Poisson ratio in Hooke's law
Data<SReal> d_young; ///< Young modulus in Hooke's law
Data<SReal> d_thickness; ///< Thickness of the elements
Comment on lines +135 to +140
Real_t<DataTypes> getPoisson() { return d_poisson.getValue(); }
void setPoisson(Real_t<DataTypes> val);
Real_t<DataTypes> getYoung() { return d_young.getValue(); }
void setYoung(Real_t<DataTypes> val);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Real_t<DataTypes> getPoisson() { return d_poisson.getValue(); }
void setPoisson(Real_t<DataTypes> val);
Real_t<DataTypes> getYoung() { return d_young.getValue(); }
void setYoung(Real_t<DataTypes> val);
SReal getPoisson() { return d_poisson.getValue(); }
void setPoisson(SReal val);
SReal getYoung() { return d_young.getValue(); }
void setYoung(SReal val);
@@ -162,26 +152,26 @@ class TriangleFEMForceField : public core::behavior::ForceField<DataTypes>
protected:

/// f += Kx where K is the stiffness matrix and x a displacement
virtual void applyStiffness(VecCoord& f, Real h, const VecCoord& x, const Real& kFactor);
virtual void applyStiffness(VecCoord_t<DataTypes>& f, Real_t<DataTypes> h, const VecCoord_t<DataTypes>& x, const Real_t<DataTypes>& kFactor);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
virtual void applyStiffness(VecCoord_t<DataTypes>& f, Real_t<DataTypes> h, const VecCoord_t<DataTypes>& x, const Real_t<DataTypes>& kFactor);
virtual void applyStiffness(VecCoord_t<DataTypes>& f, SReal h, const VecCoord_t<DataTypes>& x, const SReal& kFactor);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: clean Cleaning the code pr: status to review To notify reviewers to review this pull-request
2 participants