Skip to content

Conversation

@pcarruscag
Copy link
Member

Proposed Changes

@patelha57 @aa-g @Nicola-Fonzi @bigfooted
I'm proposing this as the way to interact with volume fields via the python wrapper.
We create a matrix view (does not copy data) into the coordinates, solution, primitives, etc. which then allows point-wise or row-wise access to the data (read or write). We can extend it to also expose derivative information.
For example:

coords = driver.Coordinates()
print(coords(iPoint, 1)) # read a value
coords.Set(iPoint, (x, y, z)) # write a row
coords.Set(iPoint, 0, coords.Get(iPoint, 0)) # set a value and different way of reading a value

solversIndices = driver.GetSolverIndices() # maps solver names to our solver integers, similar to what we do with markers
solution = driver.Solution(solverIndices["C.FLOW"]) # same object type as coords

primitiveIndices = driver.GetPrimitiveIndices() # maps primitive names to their indices.
temperatureIndex = primitiveIndices["TEMPERATURE"]
primitives = driver.Primitives()
print(primitives(iPoint, temperatureIndex))

PR Checklist

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@bigfooted
Copy link
Contributor

Oh, that's great! I like this way to get the solver index. This would also clean up the flamelet PR, that has a hacky pywrapper implementation for creating an initial solution for the flamelet solver.

Copy link
Contributor

@bigfooted bigfooted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, if no more changes are required for this PR, let's merge it.

@bigfooted
Copy link
Contributor

maybe @patelha57 @aa-g have additional comments? Else let's merge, LGTM.

@pcarruscag
Copy link
Member Author

I talked with @Nicola-Fonzi and I'll try to use the same "matrix view" strategy for data at markers. But if the current solution unblocks the flamelet work go ahead and merge this.

@bigfooted
Copy link
Contributor

Ok, thanks. I will merge this now, then clean up the flamelet branch, so it will be ready for review next week.

@bigfooted bigfooted merged commit 5d13417 into develop Mar 1, 2023
@bigfooted bigfooted deleted the python_access_solvers branch March 1, 2023 09:45
@pcarruscag pcarruscag restored the python_access_solvers branch March 5, 2023 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants