Merged
Conversation
Contributor
laetitia-m
commented
Feb 10, 2023
- Read ls and metric fields in vtk input files
- Fix some bugs on fields name in vtk output files
- Add 2D, 3D and surf tests
Merge from upstream develop
… feature/ls-vtk-input Pull latest update in my branch
Latest commits
… feature/ls-vtk-input
… feature/ls-vtk-input
…vided in command line in ls mode
Add latest mmg main commits
|
|
Algiane
reviewed
Feb 13, 2023
Member
Algiane
left a comment
There was a problem hiding this comment.
Hi @laetitia-m ,
Thanks for this PR and this great job: it works smoothly.
However, as this PR modifies public APIs and touches I/Os, I prefer to discuss some specific points with you before merging it:
- the possibility to preserve the API of the
MMG<X>_loadVt<x>Mesh_and_allDataand to store all fields in the same variable (instead of storing the metric in a dedicated variable). For this point, we might need to create a dedicated test to make sure the code behaves as I think it does; - the order of the different
MMG5_pSolvariables in the prototypes of theMMG<X>_loadVt<x>Meshfunctions (it seems more logical to me to have the metric before the level-set).
I have added few comments to some of the file modifications (without repeating similar comments). It can serve as basis for a future face-to-face discussion.
…esh_and_allData to preserve API behaviour
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.