[GeoMechanicsApplication] Add json formatting exception list#14111
Conversation
There was a problem hiding this comment.
The idea of this list is, that we can slowly format the old files by removing a few lines of this exception file, while new jsons will be formatted automatically
WPK4FEM
left a comment
There was a problem hiding this comment.
Hi Richard.
Getting to a uniform formatting is a good thing, that hopefully does not lead to longer and more verbose files. I have no clue if my remarks are even feasible with the tools at hand. Please have a look at the remarks, but wait for reactions of others before taking actions.
Thank you, Wijtze Pieter
| "IGNORE_UNDRAINED": true, | ||
| "YOUNG_MODULUS": 1e+04, | ||
| "POISSON_RATIO": 0.0, | ||
| "DENSITY_SOLID": 2.65, | ||
| "DENSITY_WATER": 1.0, | ||
| "POROSITY": 0.36, | ||
| "BULK_MODULUS_SOLID": 1e+09, | ||
| "BULK_MODULUS_FLUID": 1.755e+05, | ||
| "PERMEABILITY_XX": 0.1521, | ||
| "PERMEABILITY_YY": 0.1521, | ||
| "PERMEABILITY_XY": 0.0, | ||
| "PERMEABILITY_ZZ": 0.1521, | ||
| "PERMEABILITY_YZ": 0.0, | ||
| "PERMEABILITY_ZX": 0.0, | ||
| "DYNAMIC_VISCOSITY": 9.81, | ||
| "BIOT_COEFFICIENT": 1.0, | ||
| "RETENTION_LAW": "SaturatedLaw", | ||
| "SATURATED_SATURATION": 1.0 |
There was a problem hiding this comment.
is it possible that to have a rule that aligns the things after the : ? ( for that it would be nice if the font has equal width characters. such that it really aligns )
| "IGNORE_UNDRAINED": true, | |
| "YOUNG_MODULUS": 1e+04, | |
| "POISSON_RATIO": 0.0, | |
| "DENSITY_SOLID": 2.65, | |
| "DENSITY_WATER": 1.0, | |
| "POROSITY": 0.36, | |
| "BULK_MODULUS_SOLID": 1e+09, | |
| "BULK_MODULUS_FLUID": 1.755e+05, | |
| "PERMEABILITY_XX": 0.1521, | |
| "PERMEABILITY_YY": 0.1521, | |
| "PERMEABILITY_XY": 0.0, | |
| "PERMEABILITY_ZZ": 0.1521, | |
| "PERMEABILITY_YZ": 0.0, | |
| "PERMEABILITY_ZX": 0.0, | |
| "DYNAMIC_VISCOSITY": 9.81, | |
| "BIOT_COEFFICIENT": 1.0, | |
| "RETENTION_LAW": "SaturatedLaw", | |
| "SATURATED_SATURATION": 1.0 | |
| "IGNORE_UNDRAINED": true, | |
| "YOUNG_MODULUS": 1e+04, | |
| "POISSON_RATIO": 0.0, | |
| "DENSITY_SOLID": 2.65, | |
| "DENSITY_WATER": 1.0, | |
| "POROSITY": 0.36, | |
| "BULK_MODULUS_SOLID": 1e+09, | |
| "BULK_MODULUS_FLUID": 1.755e+05, | |
| "PERMEABILITY_XX": 0.1521, | |
| "PERMEABILITY_YY": 0.1521, | |
| "PERMEABILITY_XY": 0.0, | |
| "PERMEABILITY_ZZ": 0.1521, | |
| "PERMEABILITY_YZ": 0.0, | |
| "PERMEABILITY_ZX": 0.0, | |
| "DYNAMIC_VISCOSITY": 9.81, | |
| "BIOT_COEFFICIENT": 1.0, | |
| "RETENTION_LAW": "SaturatedLaw", | |
| "SATURATED_SATURATION": 1.0 |
Further for doubles in scientific notation, I would like if at least the . is present 1e+04 --> 1.e+04 or even nicer 1.0e+04
There was a problem hiding this comment.
I don't think json writing does the alignment in general. We could have a look how difficult it would be to implement it.
For the double in scientific notation, I'll have a look, that shouldn't be difficult
| "problem_name": "apply_initial_uniform_stress_field", | ||
| "start_time": 0.0, | ||
| "end_time": 1.0, | ||
| "echo_level": 1, | ||
| "parallel_type": "OpenMP", | ||
| "number_of_threads": 1 | ||
| }, |
There was a problem hiding this comment.
Also in the ProjectParameters.json files, some alignment after the : would be nice.
| "DENSITY_SOLID": 2.242609582059123e+03, | ||
| "DENSITY_WATER": 1.019367991845056e+03, |
| "PERMEABILITY_YY": 1.5041e-12, | ||
| "PERMEABILITY_XY": 0.0, | ||
| "DYNAMIC_VISCOSITY": 0.0013, | ||
| "DYNAMIC_VISCOSITY": 1.3e-03, |
| "UMAT_PARAMETERS": [ | ||
| 4.998729486706428e-02, | ||
| 0.19999260891644746, | ||
| 1.999926089164475e-02, | ||
| 8.64e+04, | ||
| 1.5 | ||
| ], |
There was a problem hiding this comment.
could be shorter if the opening and closing [ ] are on the lines of the first and last item of the list.
| "nodal_results": [ | ||
| "TOTAL_DISPLACEMENT", | ||
| "WATER_PRESSURE", | ||
| "CAUCHY_STRESS_TENSOR", | ||
| "TOTAL_STRESS_TENSOR" | ||
| ], |
There was a problem hiding this comment.
also here is having [ "TOTAL_DISPLACEMENT]",
...
"TOTAL_STRESS_TENSOR"],
shorter and ( for me ) clearer.
There was a problem hiding this comment.
We could make that work, I'm curious if others have the same opinion. Personally, I don't mind the two extra lines that much, so for simplicitly I'd prefer to keep it as it is, but if the team feels it's better we can of course do it
There was a problem hiding this comment.
I like the way it is now slightly more than what @WPK4FEM suggests, but it's really a matter of taste. I'm curious what the others think. I'm okay with either way.
There was a problem hiding this comment.
Okay, then for now I propose to merge it as-is. We can always change it when we change our minds
indigocoral
left a comment
There was a problem hiding this comment.
Thanks for this much-needed JSON formatting. I only have some personal opinions about it.
| "IGNORE_UNDRAINED": true, | ||
| "YOUNG_MODULUS": 1e+04, | ||
| "POISSON_RATIO": 0.0, | ||
| "DENSITY_SOLID": 2.65, | ||
| "DENSITY_WATER": 1.0, | ||
| "POROSITY": 0.36, | ||
| "BULK_MODULUS_SOLID": 1e+09, | ||
| "BULK_MODULUS_FLUID": 1.755e+05, | ||
| "PERMEABILITY_XX": 0.1521, | ||
| "PERMEABILITY_YY": 0.1521, | ||
| "PERMEABILITY_XY": 0.0, | ||
| "PERMEABILITY_ZZ": 0.1521, | ||
| "PERMEABILITY_YZ": 0.0, | ||
| "PERMEABILITY_ZX": 0.0, | ||
| "DYNAMIC_VISCOSITY": 9.81, | ||
| "BIOT_COEFFICIENT": 1.0, | ||
| "RETENTION_LAW": "SaturatedLaw", | ||
| "SATURATED_SATURATION": 1.0 |
There was a problem hiding this comment.
Just a question from my side: Why did the previous format need changing in this case?
I liked that we have all the values in a straight line/column. Made it easy for me to read the values... But I'm just curious...
There was a problem hiding this comment.
It didn't have to, I just don't know at this point how to make sure the formatting does alignment correctly. The default python json writer doesn't seem to have that option, but we could have a look into how difficult it would be to do so.
| "buffer_size": 2, | ||
| "echo_level": 1, | ||
| "clear_storage": false, | ||
| "compute_reactions": true, | ||
| "move_mesh_flag": false, | ||
| "reform_dofs_at_each_step": false, |
There was a problem hiding this comment.
Also here the alignment seemed more neat in my opinion...
| "newmark_theta": 0.5, | ||
| "strategy_type": "newton_raphson", | ||
| "convergence_criterion": "residual_criterion", | ||
| "residual_relative_tolerance": 1e-04, |
There was a problem hiding this comment.
Below is how I always do it. But it's really personal preference...
| "residual_relative_tolerance": 1e-04, | |
| "residual_relative_tolerance": 1.0e-04, |
There was a problem hiding this comment.
I agree, I'll have a look, shouldn't be too difficult to fix
| "UMAT_PARAMETERS": [ | ||
| 4.998729486706428e-02, | ||
| 0.19999260891644746, | ||
| 1.999926089164475e-02, | ||
| 8.64e+04, | ||
| 1.5 | ||
| ], |
indigocoral
left a comment
There was a problem hiding this comment.
Thanks for adding the .0 to the values. I have no further remarks on this. Hopefully in the future we can come up with a format that does keep the alignments, as that was more neat and easy to read in my opinion.
Thank you!
| "Variables": { | ||
| "IGNORE_UNDRAINED": true, | ||
| "YOUNG_MODULUS": 1e+04, | ||
| "YOUNG_MODULUS": 1.0e+04, |
There was a problem hiding this comment.
Nice! Thanks for this.
avdg81
left a comment
There was a problem hiding this comment.
Hi Richard,
Thanks for preparing this PR will help us to enforce a consistent formatting of the JSON files. I have only one suggestions and some compliments 👍
| "residual_relative_tolerance": 1.0e-04, | ||
| "residual_absolute_tolerance": 1.0e-09, |
There was a problem hiding this comment.
I like this notation more than what it used to be 👍
| "time_stepping": { | ||
| "time_step": 1, | ||
| "max_delta_time_factor": 86400000000 | ||
| "max_delta_time_factor": 8.64e+10 |
There was a problem hiding this comment.
Much more readable 👍 Thank you.
| "nodal_results": [ | ||
| "TOTAL_DISPLACEMENT", | ||
| "WATER_PRESSURE", | ||
| "CAUCHY_STRESS_TENSOR", | ||
| "TOTAL_STRESS_TENSOR" | ||
| ], |
There was a problem hiding this comment.
I like the way it is now slightly more than what @WPK4FEM suggests, but it's really a matter of taste. I'm curious what the others think. I'm okay with either way.
There was a problem hiding this comment.
I would suggest to use Unix-style path separators rather than Windows-style ones, to avoid misinterpretation of characters (for instance, \n may be understood as a newline character, whereas /n will not).
avdg81
left a comment
There was a problem hiding this comment.
I feel this PR is good to go. 👍
📝 Description
This PR adds a list of almost all json files as exceptions for our new formatter. I removed a few files from here, such that the result of the new functionality, added to
kp formatis clear and show what formatted jsons will look like