KratosMultiphysics / Kratos

Kratos Multiphysics (A.K.A Kratos) is a framework for building parallel multi-disciplinary simulation software. Modularity, extensibility and HPC are the main objectives. Kratos has BSD license and is written in C++ with extensive Python interface.
https://kratosmultiphysics.github.io/Kratos/
Other
1.02k stars 244 forks source link

Adding support for JsonPointers #11428

Open RiccardoRossi opened 1 year ago

RiccardoRossi commented 1 year ago

Description let's imagine that we have the following

{
   "problem_data" : {
        "start_time" : 0.0,
        "end_time" : 1.0
    }
    "other_settings" : {
         "end_time" : 1.0
    }  
}

where for any reason "end_time" is needed in two blocks of the json. In essence i would propose to replace that code by something like

{
   "problem_data" : {
        "start_time" : 0.0,
        "end_time" : 1.0
    }
    "other_settings" : {
         "end_time" : { "@json_pointer" : "/problem_data/end_time" } 
    }  
}

so that one only needs to input the "end_time" once and the value is propagated to the rest of the code.

In here i used the standard definition of "json pointer" as described here

"LINK"

In the design i propose this json pointers would be resolved ONLY when the json is parsed from a string, moment at which we would replace it by the actual value.

as a matter of fact, i would also add to the interface a function "GetPointer" so that one could write

my_json.GetPointer("/problem_data/end_time")

instead of

my_json["problem_data"]["end_time"]

however this is obviosuly not too urgent

matekelemen commented 1 year ago

we can already do this with @include_json (available since #10004), though it expects another file, not a value in the current json.

Edit: your example would look like this: TimeDomain.json

{
    "end_time" : 1.0
}

ProjectParameters.json

{
   "problem_data" : {
        "start_time" : 0.0,
        "@include_json" : "TimeDomain.json"
    }
    "other_settings" : {
         "@include_json" : "TimeDomain.json"
    }
}
RiccardoRossi commented 1 year ago

yes, i used the same syntax on purpose, but this is a pointer to an entry within the same json

RiccardoRossi commented 1 year ago

also note that in a sense here you replace a "value" instead of the key

matekelemen commented 1 year ago

well, if @include_json won't do and you definitely need a "pointer", there will be a couple of behaviour/implementation questions that need addressing:

What happens when the referenced value changes at runtime? Pointer semantics dictate that the pointers must also represent the updated value. This means that we cannot resolve pointers at construction time (unlike @include_json, in which case we just copy everything from the include during construction), and always fetch the referenced value dynamically.

This also means that we must extend each getter in Parameters (GetInt, GetString, etc.), checker (IsInt, IsString, etc.) and setter (SetInt, SetString, etc.) to account for cases when the value is not the expected type, but a pointer.

Furthermore, circular/invalid pointers would only get caught when pointers get accessed the first time, and not during construction.

RiccardoRossi commented 1 year ago

Hi @matekelemen first of all let me comment that i did not invent this. Json pointers are actually standardized as shortcuts to values (read it as "ways to access to locations within the json")

yet your point is valid. my intention was to avoid any runtime cost other than in the initial parsing. To achieve this i did not want the "pointer" to behave as a actual pointer (possibly we should then call it @copied_value or something of the sort) but only as a way for someone to refer ONCE to another location of the code.

that is, after parsing,

{
   "problem_data" : {
        "start_time" : 0.0,
        "end_time" : 1.0
    }
    "other_settings" : {
         "end_time" : { "@json_pointer" : "/problem_data/end_time" } 
    }  
}

would become in memory

{
   "problem_data" : {
        "start_time" : 0.0,
        "end_time" : 1.0
    }
    "other_settings" : {
         "end_time" : 1.0
    }  
}

if after the initial parsing you change the "end_time" in the root, the change would NOT be reflected into the other "end_time".

matekelemen commented 1 year ago

Alright; if we don't have to look up the values dynamically and the name reflects that, I don't see any problems with it.