sofa-framework / sofa Goto Github PK

Trying to list automatable tests we could/should implement.
Feel free to complete it!

• ProjectiveConstraint:
  • "projectResponse" could be tested by finite differences on "projectPosition"
• MultiMapping:
  • "getK" (assembled API) could be compared to "applyDJT" (non assembled API)
• ForceField:
  • masks: all the non-null lines of the stiffness matrix should have an entry in the mask vector (conservative test, i.e. the mask can contain more entries)
  • the compliance matrix should be equal to the inverse of the stiffness matrix (maybe with a different sign)
• {Multi}Mapping
  • masks: all the non-null columns of the Jacobian matrices should have an entry in the parent mask vectors (conservative test, i.e. the masks can contain more entries)

Hi,

The current dash board does not allows to differentiate tests that fails because they are 'showing' a not yet fixed sofa defect to the one caused by regression (ie tests that were supposed to work but don't anymore).

This is a problem because while introducing new regression failure is a bad thing, introducing new tests that exhibit sofa defect is a positive thing. But they have the same results...turning the dashboard to orange.

I see two consequences:
- when there is test failures because of open-issues we will detect much less easily the regressions tests (that are 'more' important !)
- people will prevent themselves to push open-issues test to avoid to turn the dash board to a not green status (because they are ashamed...or because they prefer to preserve the readability of the dash board to detect regression)

So would it be possible to change that so that each tests with a pattern (lke "OpenIssue" in their name are counted separately ?

This would allow to reports two numbers instead of one eg:
regression test failure : 0
open issues tests failure: 123
And also to let the dashboard green for open issues tests failure.

DM.

Whatever I set doUpdate to true or false the BoxROI object seems to update the selected triangles so I wonder what is supposed to be the behavior of this data field ?

Reading a JPEG file is implemented by ImageQt, which obviously depends on Qt.
It means that Qt is needed to load any JPEG image (actually, also gif, and other exotic formats).

1. I think we should be able to read JPEG files without Qt ; perhaps using libjpeg ?
2. Moreover, we need to have a Qt Gui to activate ImageQt ; then it could be an idea to move this class to a plugin ? Which will be able to read all image formats the Qt library can read ?
3. (BUG) JPEG files read by ImageQt seem to be upside-down, if we compare it with the same image converted in the PNG format (I will write a test to confirm that)

giving a texture in .png to an OglModel is working, while giving the same texture compressed in .jpg is not (the texture is loaded but it does not appear correctly on the mesh, like if the texture coordinates were incorrect)

Details

If the option in the CMake file SOFA_FLOATING_POINT_TYPE is set on float only. Compilation fails initially in SofaDenseSolver library due to errors with the Newmat library, which propagates in further dependencies. It seems that there is no float version of the methods inside Newmat.

Build output sample

• 3> Creating library C:/.../lib/Debug/SofaDenseSolver_d.lib and object C:/.../lib/Debug/SofaDenseSolver_d.exp
• 3>LULinearSolver.obj : error LNK2001: unresolved external symbol "public: virtual float __cdecl NEWMAT::BaseMatrix::SumSquare(void)const " (?SumSquare@BaseMatrix@NEWMAT@@UEBAMXZ)
• 3>NewMatCGLinearSolver.obj : error LNK2001: unresolved external symbol "public: virtual float __cdecl NEWMAT::BaseMatrix::SumSquare(void)const " (?SumSquare@BaseMatrix@NEWMAT@@UEBAMXZ)
• 3>initDenseSolver.obj : error LNK2001: unresolved external symbol "public: virtual float __cdecl NEWMAT::BaseMatrix::SumSquare(void)const " (?SumSquare@BaseMatrix@NEWMAT@@UEBAMXZ)
• 3>LULinearSolver.obj : error LNK2001: unresolved external symbol "public: virtual float __cdecl NEWMAT::BaseMatrix::SumAbsoluteValue(void)const " (?SumAbsoluteValue@BaseMatrix@NEWMAT@@UEBAMXZ)

Hi,

I found the usage of data alias more annoying than helpful.
Currently data alias are used to provide backward compatibility with old scenes. The problem is that by doing so it does not indicate to the user that the data field name has changed and that it should change to fix his scenes. The consequence is that we have a lot of old scene that are still using differents name for the same data field.

I would advocate for a more helpful (to the user point of view) approach. This approach consists in saying that the field it is using is now deprecated and provide suggestions to fix his scene. In the following I provide a template of code (taken from the soft-robot plugin) to implement this kind of behavior.

CableModel<DataTypes>::CableModel(MechanicalState* object)
    : InverseProblemConstraint<DataTypes>(object)
    , d_indexDeprecated(initData(&d_indexDeprecated, "index",
                                 "Deprecated, must be replaced by the field name 'indices'"))
    , d_indices(initData(&d_indices, "indices",
                         "List of points connected by the cable (from extremity to actuated point). \n"
                         "If no indices are given, default value is 0. \n"
                         "In case of multiple indices, one point will be actuated \n"
                         "and the others will represent sliding points for the cable."))
{
      d_indexDeprecated.setDisplayed(false);
}


template<class DataTypes>
void CableModel<DataTypes>::init()
  if(d_indexDeprecated.isSet()) {
           msg_warning(this) << "The field of the Cable component named 'index' is now deprecated. "
                                               "To remove this error message, the field 'index' should be replaced by the field 'indices'." ;
     .... depending on whether you want  to force the user to fix his scene (so quitting the init with an error) or if you want a transition phase accepting the two field you just have to duplicate the content of  d_indexDeprecated into d_indices. 
    }

If anyone has an opinion or other approach to the problem it is more than welcome.

D.

A lot of scenes without an OglModel does not display anything because the bounding box is not computed by any component. Therefore, the default value is extremely small (default value of double I think).
This behavior is mainly found with the default QtViewer GUI, as QGLViewer seems to have a reasonable default bounding box value.

Even if there will be an other GUI, we have to implement computeBBox method in as many components as with we can.

The tendency would be to alleviate Data, but one important behavior is missing.

Do you have suggestions on how to perform a listener/observer mechanism on Data w/o adding extra cost?
The idea is to be able to call a callback as soon as a Data is modified. The main usage is the gui, to be able to update some stuff w/o having to check for modifications of tons of Data at regular intervals.
This has to be done only for a few specific Data, and should not add extra cost on the 99.99% regular use cases.

The easiest way to implement such a thing would be to add a list of listeners to every Data.
But it means storing extra stuff in each Data, and checking if listeners are present at each modification of the Data.
Do you see a less intrusive way to implement such mechanism?

In sofa, some component allows to specify colors by writing color="1,0,0" and "red" while others just allow color="1,0,0".

This is a consistency issue (very very bad for the user experience).
So we should unify this in all sofa for the next release.

DM.

Hi,

several tests were already done for topological modifier, they are tested through scenes in examples/Components/Topology/TopologicalModifiers

They test: add/remove of topological elements

• Is sofaVerification still used?
• I can easily rewrite those tests in c++ using the ::Sofa_test api. Is it better? what is the politic between doing tests through scene or Sofa_test.

Even if not activated, SofaTest finds the SofaPython package
in SofaTest/CMakeLists.txt
35: if(SofaPython_FOUND)
is true whereas it should be false
and cmake exits with an error at
52: target_link_libraries(${PROJECT_NAME} SofaPython)
since SofaPython target does not exist

Windows, cmake 3.6.2

Main discussions

Stéphane: building matrices, vectors, to work with SciPy
Matthieu: Numpy is able to access the memory directly (there are examples)

There are some Python modules in Compliant with examples to assemble matrices, etc.

Components in Python (Stéphane)
"Trash" code, not clean, way to go to C++

Forcefield

Need to put breakpoints to interact with the scene from Python console

Our Python needs:

• Create scene easily
• Prototype components
• Matlab aspect

Matthieu: 90% of Datas are binded in Python, special cases need a binding.

To sum up, we already have what we need, we just don't know how to work with everything.

Besides technical aspects, what we need is a step to step tutorial and more examples.

--> Update online documentation to add Numpy
--> Add a doc for newcomers: right now it is very hard to enter in SOFA with Python (no doc)

Start with step by step tutorials and create a Python version of each step.

Other discussions: performances

Pypy

x100 perf but big big cost (recode everything: plugin, bindings, ...)

Cython

ability to code in Python and get C++ code generation
we can keep the current plugin but we wont gain perf
optimizes some small algos
ability to manually release the GIL (lock)
ability to connect to the current VM
adds a step of compilation using "CIMPORT" but it must have compilo + headers and libs Sofa

Problem of version 2.7 vs 3

Current plugin renamed SofaPython27
SofaPython passes python 3.0

How do you get to use Python?

Stéphane: Making XML optional?
Matthieu: python is not just a loader, there is a binding too
As said before, we have to use more Python in the tutorials (doc, website ...)

Referee:
@bcarrez

Interested people:
@matthieu-nesme
@damienmarchal
@etienneschmitt
@bruno-marques
@chpaulus

If you are interested please tell it.

Referee: @matthieu-nesme @ChristianDuriez

Members: @JeremieA @francoisfaure @courtecuisse, Eulalie Coevoet, Igor Peterlik

Main objective: build or compute the mechanical system when forcefields, constraints etc... are under mapping

1 implementations available using Compliance plugin (and EigenMatrix) and masks
1 implementation todo using sparseMatrix of SOFA without masks. For that, the fact that we remove the particular case of InteractionForceField could greatly simplify the solution.

There are many different cases depending on the number of dofs that are concerned by the mapped values... Difficult to have the ideal implementation for all the case, but we need to allow several strategies.

Maybe possible to avoid the "explicit" use of masks given the knowledge of the sparsity of the matrices...

Subtasks:

• Make option "use Mask" to false by default
• Tag "InteractionForceField" as deprecated
• First implementation for the sparseMatrix of SOFA using applyJt(SparseMatrixDeriv => not ideal for all the case
• see if we could define a common strategy without the use of masks.

In runSofa, on Linux, Mouse picking is working on Collision models but when picking directly on a MechanicalObject point, it becomes totally unstable.

I noticed several flaw (or hard to understand) with the way mass and totalMass interact in DiagonalMass compared to what is done in UniformMass.

eg:

• totalMass is set as readOnly in DiagonalMass while it is not in UniformMass
• despite totalMass is readOnly the value can still be changed in the scene file (so I wonder that the readOnly flag should be renamed to something that reflect what is it used for :))
• despite totalMass is readOnly there is no error message indicating to the user that the value he gave in his scene is simply dropped and that he should not use this flag.
• the totalMass parameter is re-computed in the un-specialized implementation of the reinit function... but the specialization for rigid/vec and so on are not forwarding to this un-specialized implementation but the to implementation in-herited from the parent. (Is this really what we want ? This look a very weird design to me and I suspect problems on this forwarding to the parent.).
• f_mass data (mass) field also seems to be a readOnly component (it seems deduced from massDensity and volume of the elements) but it is not tagged in the same way as totalMass. Why ?

Any feedback or confirmation/or not on these topics would help me to write the correct test.

DM.

Dear all,

I used a deliberately provocative title to start a friendly technical discussion about the way Data are initialized.

As of now, Data constructors potentially incur a non-negligible overhead per component created, and it is unclear why users should have to pay for (at least some of) it. The overhead can get pretty large as many components are created (think of contacts, for instance), so I hope this discussion will clarify what is really needed and what can be optimized away.

The current way to initialize member Data in a component is to do something like:

MyComponent() 
   : my_data( initData(&my_data, init_value, "friendly name", "help message") ) { 
 // ...
}

At this point one can remark that most information provided to initData depends on the component class, and not on the component instance being constructed, save for the initial value and the pointer to the current instance (used by the data as its 'owner').

Now what happens when the data is constructed using initData?

1. some BaseInitData objet is created, mostly holding pointers to character strings and owner,
2. BaseData is constructed from BaseInitData (BaseData.cpp:70) where the following happens:
   1. data links for inputs and outputs are setup, 2x std::vector::push_back
   2. data are added back to their owner (from which they come anyway) 1x std::vector::push_back + 1x std::map::insert for aliases

There is also the DDGNode base class initialization, which also incurs at least initLink for members inputs and outputs (cf DDGNode.cpp:43), each again causing addLink back to the owner and an extra std::vector::push_back. Phheew.

Now you probably see where I am heading at: do we really need to pay a worst case 6x heap allocation for every single data in every single component created? Or at least, can this cost be alleviated somehow?

Are there component examples that manage Data which are not known at compile-time ?

If most/all the use-cases are in fact per-class, it is relatively easy to imagine a system that maps names to instance members constructed statically, so that component instances don't have to hold and allocate a vector of pointers to their own datas (!).

Data links are more tricky since they depend on the graph, but my opinion is that there should be at least a way to disable automatic link management when they are not needed.

Now thanks for reading this far, I look forward to hearing the community's opinion on the subject :-)

MessageDispatcher::clearHandlers() can have side effects on other tests in same suite.
See #83 for error case.

SofaTest API should ensure that each test starts with a clean MessageDispatcher containing always the same set of MessageHandlers.

I need to save the scene edited with runSofa in a readable way. But the current implementation of the saving is not working because too much data are saved.

This seems to be related with the fact that the attributes of Data (Persitant, Hide, Set) are not used in a consistent way within sofa and runSofa.

So I wonder if we have a document where the different attributes behavior of the Data are specified ?

There is no implementation of FileMonitor on OSX and Windows, making all the FileMonitor.* tests to fail on these platforms (7 test fails on OSX, 8 on Windows)

This has to be fixed.

Hello. I made a test with the following scene. It actually pass in the current testing framework despite crashing upon loading in the GUI mode.

<?xml version='1.0'?>
<Node 	name='Root'>
    <VisualStyle displayFlags='showForceFields'/>
    <Node name='FEMnode'> 
        <MechanicalObject/>  
        <TetrahedronFEMForceField/> 
    </Node>
</Node>

Is there a real reason to have duplicate code between
MeshSTL and MeshSTLLoader

idem for
SOFA_LINK_CLASS(MeshOBJ)
SOFA_LINK_CLASS(MeshTrian)
SOFA_LINK_CLASS(MeshSTL)

What is the final decision?

Edit: BTW how do we add a label to github issue? I wanted to put this one under "discussion"

Could we have a dashboard compiling sofa on a ARM architecture?!

const char* BaseObjectDescription::getAttribute(const std::string& attr, const char* defaultVal=NULL);
could be replaced by
const std::string* BaseObjectDescription::getAttribute(const std::string& attr, const std::string* defaultVal=NULL);

to remove a few string creation each time getAttribute is called.

I think we should rename these components:
PlaneROI -> OrientedBoxROI
BoxROI -> AlignedBoxROI

Because if I understand correctly what PlaneROI does, it is not what we expect from the component name.

Does the files in sofa/modules/SofaVolumetricData/CMakeLists.txt really require FlowVR to compile?

Since drawQuad (as well as drawTriangle..) is a public function, one could think that we can call it from any draw function and get a Quad drawn. But drawQuad doesn't call glBegin(GL_QUADS) which than fail to draw the quad. We must call ourself glBegin(GL_QUADS) or simply call drawQuads with a vector of 4 vertices.

Should drawQuad function (as well as drawTriangle..) be made private? Or maybe add a boolean parameter in drawQuad to disable the glBegin calls when we are drawing more than one quad?

DataFileNameVector is a Data<vector> where the strings are separated by spaces in the string-serialization. So how to include spaces in one of the paths? (right now it is splitting this path into two separated paths).

couldn't mini boost just be removed from the source tree now ?

Issue with this exporter when exporting every N steps.
And export only the visual model, not the mechanical object.

• fix the export every N steps
  

• export the mechanical object of the node
  

• create an additional component exporting the visual model (based on what was written before)
  

This issue is connected to the discussion:
aee0d91

This discussion was continued between I, Mathieu and Guillaume, during our weekly meeting.

Our conclusion were that:

• in sofa::core the defaultmessagehandler should be the simplest so that people making third party applications out of Sofa are not disturbed by the new formatter.

• runSofa & Test may use the RichStyleFormater

• it would be nice to add more capability of routing the messages (explaination and details in the following).

New features:
Currently the msg* API has two class of message (Dev and Runtime). Message of class Runtime are supposed to be for "the users". Message of class Dev are supposed to be for developers.
You can send message of class Runtime with msg_info()...and message of class Dev with dmsg info(). The use of macro was to allows us to totally remove the dmsg* from the sofa official releases (they are totally removed from the binary and thus have a zero cost).

In addition to the classes of message is the 'type' which is among: Advice, Info, Warning, Error, Deprecated, Fatal. To select the type you can use the macro msg info, msg advice, msg error, ... or their dmsg equivalent.

It was suggested:

• to add one extra class of message for "Log"
• to add a new MessageHandler that can route precisely the messages depending on the developper's need.
  (eg:
  - route all the message to a FileMessageHandler but not the one of type advice)
  - route all the messages of class Log to a FileMessageHandler as well as the class Runtime&Error)

DM

Some components in SOFA are very close.
For example, we have 5 versions of TetrahedralFEMForceField.
We should clean this.

Referee: @IPeterlik

Members: @IPeterlik @remibessard @hdeling @damienmarchal @fredroy @epernod Jeremie

Main objective: provide a continuous support for validation of SOFA

Subtasks and estimated duration

• using python, based on Method for Manufactured Solutions (MMS), static linear elasticity with linear FEM in (compression and torsion with < 5% deformation)
  ex: TensorMassFEMForceField
• TensorMassFEMForceField : addKToMatrix, vector of E
• extend it to beams (BeamAdapter)
• implement engine : plugged to compute stress data in all FEM in SOFA

Future

• write article
• contact Stefan Suwelack for integration into CAEML.org

Validation solution:

• analytical
• software comparison (Abaqus)
• tests
• experimental setup

cases:

• dynamic vs static
• linear elasticity with linear FEM
• metrics : displacement, stress+strain tensors

Hi all,

One test related to LightManager is failing only on windows eg:
6d3e139

The failing scene is rather simple:

<Node name='Root' gravity='0 -9.81 0' time='0' animate='0' >
    <Node name='Level 1'>
        <MechanicalObject template='Vec3d'/>
        <LightManager name='lightmanager'/>
    </Node>
</Node>

Any one know why the test on Windows behave differently to to other plateforms ?

DM.

To reproduce the bug:

• Launch sofa with any scene
• Hit the "Save screenshot" button and save the image somewhere
• Hit again the "Save screenshot"
• The gui should now be frozen

Sofa version is 178a7b1 (current master branch)
Linked against Qt 5.5.1 and boost 1.58.0
Compiled with clang 3.8.0
Ubuntu 16.04.1 LTS

We need to update the documentation to describe the new features.

It would be easier to notice a typo in a Data name.
(I think this is done during xml parsing)

In sofa we are missing a way to handle the component when some failure happens (eg: when it cannot be initialized). Several code patterns are possible... more or less invasive...

I'm searching for different alternatives solutions I could employ to solve this issue.

DM.

FileRepository needs to be checked and fixed.

It does not work for sure in some situations, e.g. if a sofa install is copied in a path containing accents on windows, FileRepository does not find the files.

Note I have not go any further, and everything needs to be investigated.
I did not test with the distributed, precompiled package. I did not test on linux...

I,

I'm factoring the code that is in double/float template specialization using the design pattern used in the Image plugin (an external class Specialization which is friend to the class to specialize).

The code in:

BilateralInteractionConstraint<Rigid3fTypes>::getConstraintResolution(...)
BilateralInteractionConstraint<Rigid3dTypes>::getConstraintResolution(...)

is different.
Float is using

  temp->tolerance = 0.0001;

Double is using:

  temp->tolerance = 0.01;

Can someone check if this is expected or if this a kind of "someone change the value in one place and forgot the other".

DM.

helper::vector, helper::set, helper::list, etc. lack of important c++11 functionalities
such as move constructor, initializer_list and maybe other stuffs.

helper::vector is adding several stuffs to std::vector, so it makes sense to add these functionalities.

But is derivating helper::set, helper::list from stl container the right design since they are simply adding the stream operators (for string serialization)? Could not we simply add these operators outside of the class, and always use std containers?

There seems to be a memory leak when using LocalMinDistance for collision detection. This can be seen if the memory usage is monitored when there is continuous collision between two objects. This has been checked with v15.12 and the latest changes from master branch.The code has been checked for LocalMinDistance as well as through valgrind tool but no result till now.

Refer to the post on the forum:
https://www.sofa-framework.org/community/forum/topic/memory-leak-when-using-localmindistance/

The specialization of the

void DiagonalMass<Rigid2fTypes, Rigid2fMass>::init() 
void DiagonalMass<Rigid3fTypes, Rigid3fMass>::init() 

is totally different to the one for:

void DiagonalMass<Rigid2dTypes, Rigid2dMass>::init() 
void DiagonalMass<Rigid3dTypes, Rigid3dMass>::init() 

This seems weird to me. Can someone confirm this is a bug (and that probably the Rigid2f/3f should be the same as Rigid2d and Rigid3d).

Hi all,

I wanted to discuss the default behavior of the friction coefficients in the contact models of the Compliant plugin (FrictionCompliantContact.h and CompliantContact.h). The current behavior seems kind of strange:

1. the global mu coefficient set in the DefaultContactManager component always overrules friction coefficients defined locally for each collision model.

2. when the global mu is set to exactly 0.0, the local coefficients get activated. negative global mu does funny but not really useful stuff in simulations

3. When having two mu defined by the two colliding models, currently they just get multiplied. So If both models have mu=0.5, they end up having a contact's mu=0.25! Same with large mu: mu=2.0 (i.e. rubber) -> 4.0.

I believe that especially the latter behavior would catch many users on the wrong foot.

So here are some changes I propose:

ad 1) locally defined mu should take precedence over globally defined mu's. Of course, this means breaking the existing behavior and probably triggers many regressions, so this should be considered carefully.

ad 2) replace mu.getValue()? with (mu.getValue()>0.) ? or (mu.getValue()>=0.) ? -> a negative value simply means disabling the global override.

ad 3) use the geometric mean of the friction coefficients: replace
this->model1->getContactFriction(0)*this->model2->getContactFriction(0)
with
sqrt(this->model1->getContactFriction(0)*this->model2->getContactFriction(0));

An alternative could be to use the smaller of the two coefficients, but I'm not sure what's the more sensible behavior.

When saving scene file either in python and xml a lot of informations are added which ruins the initial file which make it very hard to read and manipulate.

Fixing this would be great.

Referee: @faichele

Members: @faichele , @fjourdes , @ChristianDuriez , @thomas-lemaire , @IPeterlik , @courtecuisse , @hdeling

Main objective: bring TaskScheduler within SofaKernel

Subtasks and estimated duration:

• merge the branch issofa_multithreading involving the MultiThreading plugin into master (fixes)
• remove dependencies to boost (might face issue in the lock implementation)
• create trivial examples describing the API
• add documentation, and examples of Intel
• migration into SofaKernel

Non-requisite:

• no maintenance guarantees for the plugin Multithreading

Future

• nice feature : experiment the definition of a pointer to the Scheduler into ExecParams
• anyone interested can join!

Hi all,

in SphereModel.cpp it is unclear to me why the functions

Vector3 TSphere<defaulttype::getContactPointByNormal( const Vector3& );
Vector3 TSphere<defaulttype::getContactPointWithSurfacePoint(...);

Are specialized on Vec3fd to return the center only.

Second question is why do the message says that SphereModel only work with a Vec3 while it seems to also accept Rigid3 types.

Anyone can explain ?

Thank,
DM.

Currently sofa does not compile with VS2013, and I see there is no VS2013 VM on the continuous integration.

• Is VS2013 support dropped ?
• I read in some email that there is a plan to have again a VS2013 VM, is it still the plan ?

thomas

The idea is to create issues for small stuff/cleans/bugfixes that could be done by anyone.