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

Model non-existant read and write directions in schema #30

Open
uekerman opened this issue Jan 29, 2025 · 4 comments
Open

Model non-existant read and write directions in schema #30

uekerman opened this issue Jan 29, 2025 · 4 comments
Assignees

Comments

@uekerman
Copy link
Member

Sometimes, read or write data does not exist. For instance, for uni-directional coupling or if multiple meshes are used.
How should we model such situations in the schema?

Option 1: Empty arrays

read_data_names is an empty array.

{
  "participant_name": "Fluid",
  "precice_config_file_name": "../precice-config.xml",
  "interfaces": [
    {
      "mesh_name": "Fluid-Mesh",
      "patches": [
        "interface"
      ],
      "location": "faceCenters",
      "read_data_names": [
      ],
      "write_data_names": [
        "Temperature"
      ]
    }
  ]
}

Option 2: Leave array out

read_data_names does not exist.

{
  "participant_name": "Fluid",
  "precice_config_file_name": "../precice-config.xml",
  "interfaces": [
    {
      "mesh_name": "Fluid-Mesh",
      "patches": [
        "interface"
      ],
      "location": "faceCenters",
      "write_data_names": [
        "Temperature"
      ]
    }
  ]
}

Remarks

  • The OpenFOAM adapter currently follows option 1, but this should not be a technical restriction from OpenFOAM, or is it? @MakisH
  • Pro option 1: More explicit. We can make both fields (read and write) required.
  • Pro option 2: Shorter. Also more intuitive? Clearer to generate? (there will presumably be no data connection in the topology description)
  • Allowing both options could be dangerous as this would then require that all adapters need to be able to handle both situations.
@MakisH
Copy link
Member

MakisH commented Jan 30, 2025

* The OpenFOAM adapter currently follows option 1, but this should not be a technical restriction from OpenFOAM, or is it? [@MakisH](https://github.com/MakisH)

Apparently, this is not a technical restriction (anymore?), and I modified the adapter to support both: precice/openfoam-adapter#348

Allowing both options could be dangerous as this would then require that all adapters need to be able to handle both situations.

I would ask for a specific one of the options, but not forbid supporting both (which I guess you are not suggesting anyway).

I think that option 2 (omitted) would be the more intuitive, given what we are used to in other contexts.

@uekerman
Copy link
Member Author

I would ask for a specific one of the options, but not forbid supporting both (which I guess you are not suggesting anyway).

Not exactly. I think it would be better to forbid the other one in the schema. Otherwise, we get interoperability issues. Imagine converting the config of an OpenFOAM case to another solver. Of course, an adapter could still support the other variant as deprecated.

@MakisH
Copy link
Member

MakisH commented Jan 30, 2025

But then this also gets annoying when one is experimenting with different options. For example, in the fluid-fluid coupling studies, we often had to toggle which fields we read and write. Having to constantly enable/disable a list because it ends up being empty sounds loosely-motivated to me. As a user, I would expect an empty list to be the same as an omitted list.

We could give warnings in the validation step that this might not work with all adapters.

@uekerman
Copy link
Member Author

Having to constantly enable/disable a list because it ends up being empty sounds loosely-motivated to me. As a user, I would expect an empty list to be the same as an omitted list.

I see the point. If a GUI is used, this could still be one click only.

We could give warnings in the validation step that this might not work with all adapters.

I am not sure if JSON schema supports such warnings.

@Logende What is your view on this? This must be a common problem in configurations, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants