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

partitioned-heat-conduction: direct mesh access #299

Merged
merged 18 commits into from
Nov 14, 2022

Conversation

gertjanvanzwieten
Copy link
Collaborator

@uekerman: over to you!

@uekerman
Copy link
Member

uekerman commented Nov 8, 2022

@gertjanvanzwieten Could you please allow me to push to your branch. That would make things easier. Thanks!

@gertjanvanzwieten
Copy link
Collaborator Author

@uekerman try now? I think you should have full rights.

@uekerman
Copy link
Member

uekerman commented Nov 9, 2022

@gertjanvanzwieten Works. Thanks!
(When opening a PR you can tick "allow push access by maintainers" or similar. Then, you don't have to give access to your complete fork, but only the branch. But I don't complain 😊 )

@uekerman uekerman requested review from IshaanDesai and MakisH and removed request for uekerman November 9, 2022 14:48
@uekerman
Copy link
Member

uekerman commented Nov 9, 2022

Ready for review.

  • @MakisH Could you please check for tutorials style
  • @IshaanDesai Could you please check the Nutils codes and run them

Motivation for this case is to showcase the direct mesh access feature, developed by @davidscn. We will add more participants here in the future (potentially a G+Smo one).

Could not be merged with the existing partitioned-heat-conduction tutorial as we need a different preCICE config.

@davidscn
Copy link
Member

davidscn commented Nov 9, 2022

I also have a higher-order solver using the direct-mesh access with this tutorial. I will also check for the convergence behavior when coupling with the Nutils code here.

@davidscn
Copy link
Member

Tested it and it works like charm, even the high-order properties are preserved (as expected).

@uekerman
Copy link
Member

Tested it and it works like charm, even the high-order properties are preserved (as expected).

What kind of tests did you do? Also different meshes left and right?

@davidscn
Copy link
Member

davidscn commented Nov 11, 2022

What kind of tests did you do? Also different meshes left and right?

deal.II uses hexahedral meshes and Nutils tet meshes, so I'm not sure what exactly you want to have more different. Maybe I should also add that I didn't do a full convergence study here, but one can clearly see the error drop when going from first order polynomials to seconds order polynomials. I can test things more thoroughly if there is any need. Right now, it was more or less meant as a consistency check.

Edit: Only thing which looks a bit confusing is the reported number of DoFs by Nutils vs the vtk output. Are higher degrees sub-sampled in the vtk output?

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.

Just a minor suggestion to structure the time loop in a way that is consistent with other tutorials.

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

I ran the case, checked the README, clean, and run scripts, and compared the precice-config.xml to the basic partitioned-heat-conduction. Apart from some minor comments, looks good! 👍

partitioned-heat-conduction-direct/README.md Outdated Show resolved Hide resolved
partitioned-heat-conduction-direct/README.md Outdated Show resolved Hide resolved
partitioned-heat-conduction-direct/README.md Outdated Show resolved Hide resolved
Co-authored-by: Ishaan Desai <ishaan.desai@ipvs.uni-stuttgart.de>
@uekerman
Copy link
Member

Only thing which looks a bit confusing is the reported number of DoFs by Nutils vs the vtk output. Are higher degrees sub-sampled in the vtk output?

Exactly, this happens here:

bezier = domain.sample('bezier', degree * 2)
...
x, u, uexact = bezier.eval(['x_i', 'u', 'uexact'] @ ns, lhs=lhs, t=t)

uekerman and others added 4 commits November 14, 2022 12:16
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
@uekerman
Copy link
Member

@MakisH Ready to merge from my side

@MakisH MakisH merged commit 782b2b1 into precice:develop Nov 14, 2022
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

Successfully merging this pull request may close these issues.

5 participants