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

Improve handling of NaN values #245

Open
hjabbot opened this issue Feb 5, 2024 · 1 comment
Open

Improve handling of NaN values #245

hjabbot opened this issue Feb 5, 2024 · 1 comment
Assignees
Labels
Bug It might be considered a bug...

Comments

@hjabbot
Copy link
Collaborator

hjabbot commented Feb 5, 2024

Moving here from project repo #83.

This could go into MeshiPhi, but I personally think that NaN's are perfectly valid in meshes, so anything downstream should handle NaN's gracefully. Issue is NaNs and Infs aren't natively supported by the JSON schema so strictly speaking they shouldn't be in the output of MeshiPhi meshes, but it seems dangerous to equate NaNs to a real value.


NaN values in the mesh shouldn't be allowed to reach the route planner as they will cause errors that may be difficult to debug. Currently NaN values are filtered out within the vessel performance class and all NaN values are set to zero. We should consider whether this is the correct approach in all cases and identify where NaN values might arise and how their propagation could be avoided.

@Ulvetanna Ulvetanna added the Feature Adding in a new feature label Feb 12, 2024
@Ulvetanna Ulvetanna added this to the 0.4 milestone Apr 10, 2024
@Ulvetanna Ulvetanna assigned gecoombs and unassigned gecoombs and SamuelHall700 Apr 10, 2024
@Ulvetanna Ulvetanna added Bug It might be considered a bug... and removed Feature Adding in a new feature labels Apr 10, 2024
@SamuelHall700
Copy link
Collaborator

Currently, all NaN's in a mesh are set to 0's by the vessel performance modeller.

With the refactoring of the route planner, we plan to remove this from the vessel performance modeller and have them handled more gracefully by the route planner. This work is still ongoing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug It might be considered a bug...
Projects
None yet
Development

No branches or pull requests

4 participants