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

Add gradient related API #145

Merged
merged 33 commits into from
Sep 1, 2022
Merged

Add gradient related API #145

merged 33 commits into from
Sep 1, 2022

Conversation

kursatyurt
Copy link
Contributor

This PR adds nearest neighbour gradient related API to python bindings

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

Looks good overall, the documentation needs some clarification. Please apply relevant comments to all API functions (didn't look at all). The docker image has been updated. As far as I understand you can add tests now and they should work.

cyprecice/cyprecice.pyx Show resolved Hide resolved
cyprecice/cyprecice.pyx Outdated Show resolved Hide resolved
cyprecice/cyprecice.pyx Show resolved Hide resolved
cyprecice/cyprecice.pyx Show resolved Hide resolved
cyprecice/cyprecice.pyx Outdated Show resolved Hide resolved
cyprecice/cyprecice.pyx Outdated Show resolved Hide resolved
@kursatyurt
Copy link
Contributor Author

@IshaanDesai @BenjaminRodenberg @davidscn Tests are ready. Since gradient data has no read API, tests are only write-type tests that checks different data structures.

cyprecice/cyprecice.pyx Outdated Show resolved Hide resolved
@davidscn
Copy link
Member

Since gradient data has no read API, tests are only write-type tests that checks different data structures.

How can we validate the correctness of the data passed to preCICE? Can we execute a basic integration test or access preCICE internals?

@IshaanDesai
Copy link
Member

How can we validate the correctness of the data passed to preCICE? Can we execute a basic integration test or access preCICE internals?

I have not looked at the tests yet but in general the idea of mock testing in the python bindings is that we provide an expected output to a mocked preCICE interface and then we pass the data and compare it against the expected (correct) data already present inside the mocked preCICE interface.

@kursatyurt
Copy link
Contributor Author

How can we validate the correctness of the data passed to preCICE? Can we execute a basic integration test or access preCICE internals?

I have not looked at the tests yet but in general the idea of mock testing in the python bindings is that we provide an expected output to a mocked preCICE interface and then we pass the data and compare it against the expected (correct) data already present inside the mocked preCICE interface.

So can I write the assertion in cpp file and expect some correct input from the test script?

@IshaanDesai IshaanDesai removed the request for review from BenjaminRodenberg July 14, 2022 13:05
kursatyurt and others added 22 commits July 14, 2022 20:39
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

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

API and tests look good now 👍 I will try to update the Spack image so that the build goes through and then we can merge this

@davidscn
Copy link
Member

@IshaanDesai is this now ready to merge?

@IshaanDesai
Copy link
Member

@davidscn @kursatyurt just sync with develop and then we can proceed with merging

@kursatyurt
Copy link
Contributor Author

@davidscn @kursatyurt just sync with develop and then we can proceed with merging

Done!.

@IshaanDesai IshaanDesai merged commit 20ff617 into precice:develop Sep 1, 2022
@IshaanDesai IshaanDesai mentioned this pull request Sep 1, 2022
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