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

Removed PolynomialMapCubedSphereDiscreteModel and PolynomialMapCubedSphereTriangulation types. #35

Merged
merged 2 commits into from
Jul 1, 2022

Conversation

amartinhuertas
Copy link
Collaborator

They are not actually needed. We can provide the same functionality using Gridap types.

PolynomialMapCubedSphereTriangulation types. They
are not actually needed. We can provide the same
functionality using Gridap types.
@davelee2804
Copy link
Collaborator

Hi @amartinhuertas , is this PR going to effect the use of GridapHybrid on the cubed sphere?

@amartinhuertas
Copy link
Collaborator Author

Hi @amartinhuertas , is this PR going to effect the use of GridapHybrid on the cubed sphere?

Hi @davelee2804 ! I was revisiting the data structures for CubedSphere meshing (towards refreshing what we did) and I realized that there are two types which are just wrappers of Gridap types. Thus, we can eliminate them. This does not solve per-se the current problem we have with GridapHybrid on the cubed sphere.

@davelee2804
Copy link
Collaborator

Thanks for the confirmation @amartinhuertas

@amartinhuertas
Copy link
Collaborator Author

For the records, MPI tests are failing. https://github.com/gridapapps/GridapGeosciences.jl/runs/7143096336?check_suite_focus=true#step:9:102

However, I realized that exactly the same error is in the hdg_adv branch. https://github.com/gridapapps/GridapGeosciences.jl/runs/7129780507?check_suite_focus=true#step:9:102

Thus, the changes introduced in this PR seem to be inocent for this failing tests.

@amartinhuertas
Copy link
Collaborator Author

For the records, the following commit in main might circumvent (temporarily) the failing MPI Tests 7cf147e

@amartinhuertas
Copy link
Collaborator Author

Ok, sequential tests pass. This seems to confirm that the two data structures I have eliminated in this PR are not actually needed.

@davelee2804
Copy link
Collaborator

Thanks @amartinhuertas , I'll pull that in...

…nto refactoring_cubed_sphere_data_structures
@amartinhuertas
Copy link
Collaborator Author

Thanks @amartinhuertas , I'll pull that in...

Let us see if the tests pass first in this branch. We have to pull that in the ADV-HDG branch with care. There might be conflicts. I can do it, no worries.

@davelee2804
Copy link
Collaborator

OK, thanks Alberto!

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2022

Codecov Report

Merging #35 (d790772) into master (7cf147e) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master     #35   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          18      18           
  Lines        1258    1246   -12     
======================================
+ Misses       1258    1246   -12     
Impacted Files Coverage Δ
src/CubedSphereDiscreteModels.jl 0.00% <0.00%> (ø)
src/CubedSphereTriangulations.jl 0.00% <ø> (ø)

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 7cf147e...d790772. Read the comment docs.

@amartinhuertas amartinhuertas merged commit 288cf35 into master Jul 1, 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.

None yet

3 participants