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

Periodic BC for cartesian grids #266

Merged
merged 4 commits into from
May 27, 2020
Merged

Periodic BC for cartesian grids #266

merged 4 commits into from
May 27, 2020

Conversation

jesusbonilla
Copy link
Collaborator

  • Added field isperiodic to cartesian descriptor
  • Modified constructors of CartesianDescriptor and CartesianDiscreteModel
  • Added new method for _generate_cell_to_vertices_from_grid in
    UnstructuredGrids.jl that implements the grid numbering for periodic BC.
  • Added test PeriodicBC.jl in Geometry tests to check proper numbering of vefs
    with periodic BC.
  • Added tests PeriodicDarcy.jl and PeriodicCoupledPoisson.jl to check both
    with periodic BC.

Minor changes are still required in CartesianDescriptor constructors (see issue #263). Apart from that, I would say it is ready to merge.

* Added field isperiodic to cartesian descriptor
* Modified constructors of CartesianDescriptor and CartesianDiscreteModel
* Added new method for _generate_cell_to_vertices_from_grid in
UnstructuredGrids.jl that implements the grid numbering for periodic BC.
* Added test PeriodicBC.jl in Geometry tests to check proper numbering of vefs
with periodic BC.
* Added tests PeriodicDarcy.jl and PeriodicCoupledPoisson.jl to check both
with periodic BC.
@jesusbonilla jesusbonilla self-assigned this May 25, 2020
@codecov-commenter
Copy link

codecov-commenter commented May 25, 2020

Codecov Report

Merging #266 into master will increase coverage by 0.05%.
The diff coverage is 97.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   88.94%   89.00%   +0.05%     
==========================================
  Files         147      147              
  Lines        9604     9646      +42     
==========================================
+ Hits         8542     8585      +43     
+ Misses       1062     1061       -1     
Impacted Files Coverage Δ
src/Geometry/UnstructuredGrids.jl 69.29% <ø> (+0.87%) ⬆️
src/Geometry/CartesianGrids.jl 91.45% <97.77%> (+3.14%) ⬆️
src/Geometry/CartesianDiscreteModels.jl 100.00% <100.00%> (ø)
src/Arrays/Tables.jl 87.05% <0.00%> (+0.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edd804b...1fb958b. Read the comment docs.

Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Please address the last comments in the issue #263 and pull to this branch again for a second review.

Jesus Bonilla added 2 commits May 26, 2020 10:19
* Converted optional arguments of CartesianDescriptor constructors to
key-word arguments.
* Added deprecated signatures for backwards compatibility.
* Uptated tests to use new signatures.
* Updated News.md.
Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

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

Hi @jesusbonilla

Please address the comments below. Thanks!

src/Geometry/CartesianGrids.jl Outdated Show resolved Hide resolved
src/Geometry/CartesianGrids.jl Outdated Show resolved Hide resolved
src/Geometry/CartesianDiscreteModels.jl Outdated Show resolved Hide resolved
src/Geometry/CartesianDiscreteModels.jl Outdated Show resolved Hide resolved
* Minor changes in CartesianDescriptor documentation
* Renamed UnstructuredGridTopology used for numbering a grid with periodic BC
to _cartesian_grid_topology_with_periodic_bcs.
* Moved above function (and related) to CartesianGrids.jl
@fverdugo fverdugo merged commit 1fde696 into master May 27, 2020
@fverdugo fverdugo deleted the add_periodic_BC branch May 27, 2020 12:01
@fverdugo
Copy link
Member

PR merged!

Now, you appear in the list of Gridap contributors. Welcome!

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.

3 participants