-
Notifications
You must be signed in to change notification settings - Fork 92
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
Save the grids #1024
base: master
Are you sure you want to change the base?
Save the grids #1024
Conversation
Is nobody thinking of the grids? TOML can save the grids!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I think having something along these lines can be helpful to users. I left some comments. Furthermore, the PR is missing test coverage and docs for the new functionality.
return t_string | ||
end | ||
|
||
export toml_saves_the_grids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Grid(cells,nodes,cellsets,nodesets,facetsets,vertexsets) | ||
end | ||
|
||
export parse_the_grids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -0,0 +1,120 @@ | |||
function parse_the_grids(path::String="/tmp/grid.toml") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this function is exported users will see it. Hence a doc string should be added for users.
end | ||
|
||
# find a small number of digits for precise storage | ||
function toml_saves_the_grids(x::AbstractFloat) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not confident that this works properly for all float types. Seems like this implementation is specific to Float64 and (more narrow types). But this function essentially generates a string of the truncated real number represenation?
if N == 1 | ||
t_string = @sprintf("%.0E",x) | ||
elseif N == 2 | ||
t_string = @sprintf("%.1E",x) | ||
elseif N == 3 | ||
t_string = @sprintf("%.2E",x) | ||
elseif N == 4 | ||
t_string = @sprintf("%.3E",x) | ||
elseif N == 5 | ||
t_string = @sprintf("%.4E",x) | ||
elseif N == 6 | ||
t_string = @sprintf("%.5E",x) | ||
elseif N == 7 | ||
t_string = @sprintf("%.6E",x) | ||
elseif N == 8 | ||
t_string = @sprintf("%.7E",x) | ||
elseif N == 9 | ||
t_string = @sprintf("%.8E",x) | ||
elseif N == 10 | ||
t_string = @sprintf("%.9E",x) | ||
elseif N == 11 | ||
t_string = @sprintf("%.10E",x) | ||
elseif N == 12 | ||
t_string = @sprintf("%.11E",x) | ||
elseif N == 13 | ||
t_string = @sprintf("%.12E",x) | ||
elseif N == 14 | ||
t_string = @sprintf("%.13E",x) | ||
elseif N == 15 | ||
t_string = @sprintf("%.14E",x) | ||
elseif N == 16 | ||
t_string = @sprintf("%.15E",x) | ||
elseif N == 17 | ||
t_string = @sprintf("%.16E",x) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just be something like
if N == 1 | |
t_string = @sprintf("%.0E",x) | |
elseif N == 2 | |
t_string = @sprintf("%.1E",x) | |
elseif N == 3 | |
t_string = @sprintf("%.2E",x) | |
elseif N == 4 | |
t_string = @sprintf("%.3E",x) | |
elseif N == 5 | |
t_string = @sprintf("%.4E",x) | |
elseif N == 6 | |
t_string = @sprintf("%.5E",x) | |
elseif N == 7 | |
t_string = @sprintf("%.6E",x) | |
elseif N == 8 | |
t_string = @sprintf("%.7E",x) | |
elseif N == 9 | |
t_string = @sprintf("%.8E",x) | |
elseif N == 10 | |
t_string = @sprintf("%.9E",x) | |
elseif N == 11 | |
t_string = @sprintf("%.10E",x) | |
elseif N == 12 | |
t_string = @sprintf("%.11E",x) | |
elseif N == 13 | |
t_string = @sprintf("%.12E",x) | |
elseif N == 14 | |
t_string = @sprintf("%.13E",x) | |
elseif N == 15 | |
t_string = @sprintf("%.14E",x) | |
elseif N == 16 | |
t_string = @sprintf("%.15E",x) | |
elseif N == 17 | |
t_string = @sprintf("%.16E",x) | |
end | |
t_string = @sprintf("%.$(N-1)E",x) |
Instead of parsing to TOML, then I think @koehlerson's idea of supporting saving to HDF5 should be easier and more efficient. |
I personally rather not add a bespoke textual format for this. I don't really see the use case. I find it best to go all out Julia which means using Serializatin stdlib or something standard like HDF5, Arrow, etc. |
Yes. All you make good arguments. I could counter them, but that annoys me. |
Perhaps it could/should live in https://github.com/Ferrite-FEM/FerriteMeshParser.jl? |
From my point of view, I'm not sure what the advantage is of having a plain text format?
There, I would suggest to follow some existing mesh format in that case.
|
Is nobody thinking of the grids?
TOML can save the grids!