Double vs. Float in MoMoIO

[BernhardEgger]

When saving a MoMo using the MoMoIO parts of the model are converted to float whilst other parts stay double. Writing and reading a model can change the model since a MoMo has stuff represented as double which is only written as float.

This causes errors e.g. when calculating landmark points (on a freshly generated model, not read from file) and adding them to the model. The points work before writing the model but the MoMoIO brakes the connection. The landmarks stay as doubles whilst the model points are converted to float.

[Andreas-Forster]

Thanks for pointing out this problem.

I agree that the current state is not acceptable. However that the model does not stay the same, i.e. is converted to floats, is intended. The decision to write the model using floats is in order to stay compatible with the Statismo-File-Format (see https://github.com/statismo/statismo/wiki/The-Statismo-File-Format).

Non the less, you are right that the model converted to float but not converting the landmarks is not an option. In the model we offer the landmarkPointId( tag: String) method which use the map Map[Point[_3D],PointId] of a UnstructuredPointsDomain. This will clearly fail if the points of the UnstructuredPointsDomain are converted to float when writing and back to double when read, but the landmarks not.

That fact that we have this function limits the landmarks to actually be an exact vertex point. While in principle we could discuss this fact, I do not think that there is much reason to drop this restriction, I would argue for converting the landmarks, as the rest of the model, to floats when writing and to double when reading as a quick fix at the moment.

[BernhardEgger]

I agree - converting the landmarks too when writing the model solves the issue (tested) and does not drop any of current functionality/compatibility.

[BernhardEgger]

Is it ok if I add a fix, that converts the landmarks too?

[Andreas-Forster]

I think it makes sense, that if we write the model in float we also store the landmarks as such. So from my side, I would be for merging such a pull request. If somebody is against it, he might comment here or on the PR to follow.

[BernhardEgger]

We wanted to change that today and realized that it is not that easy. The saving of landmarks works via scalismo and the LandmarkIO there. This landmarkIO reads and writes in double. So I don't think we can change it locally in an elegant way. What would you propose? Is it feasible to change the landmarkIO to float - I assume not, since meshes are not saved in float. An alternative would be to do a nearestneighbour search when loading the model for landmarks that are not defined and closer than a certain epsilon? Or change the whole model writing to float - how much would the filesize actually grow?

[Andreas-Forster]

For me it is impossible to:

• require that LandmarkIO write only floats.
• to load the model and move the landmarks to vertices when their written location is then different.

Possible soltions I can think of:

• We could of course write our own LandmarkIO to address the double/float thing.
• We could change the models .withLandmarks method to check and adapt landmarks.

The most direct and maybe best workaround I see is changing the workflow, such that one saves and loads the model first once before defining landmarks.

While thinking about it, the concept of a landmark might be more general than a limited option of only selecting vertices. So I am unsure if we really should enforce that. Maybe we can offer a helper function within scalismo-faces that for a given model and a set of landmarks both are converted to Float and the landmarks are enforced to the next closest vertex.