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

Updating Compat Section #69

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Updating Compat Section #69

wants to merge 5 commits into from

Conversation

hammy4815
Copy link
Contributor

See #62 (comment)

My working dependency versions:

  [13f3f980] CairoMakie v0.11.8
  [5789e2e9] FileIO v1.16.2
  [1a297f60] FillArrays v1.9.3
  [e9467ef8] GLMakie v0.9.8
  [5c1252a2] GeometryBasics v0.4.10
  [56d4f2e9] Gridap v0.17.23
  [ee78f7c6] Makie v0.20.7

@hammy4815
Copy link
Contributor Author

I think the issues above were caused from trying to compile the docs (which calls GridapMakie and then Gridap dependencies) with Julia v1.6 instead of 1.9, so I just pushed an update to try and fix the issue.

@HelgeGehring
Copy link
Contributor

Yay, great to see an update 🥳
Could we directly add julia 1.10 as well?

@fverdugo
Copy link
Member

Hi @hammy4815,

the ci failed for this reason:

"This request was automatically failed because there were no enabled runners online to process the request for more than 1 days."

This is the first time I see this problem. Any idea?

@HelgeGehring
Copy link
Contributor

HelgeGehring commented Feb 27, 2024

We probably just need to update ubuntu-18.04 which is not supported anymore.
In #71 I made a pull request to update it to ubuntu-latest.

@fverdugo
Copy link
Member

Hi @hammy4815, the PR #71 has been merged. Can you pull this changes to your fork, and then update this PR?

@hammy4815
Copy link
Contributor Author

The only tests that are failing are:

fig, _ , sc = plot(Γ, uh, colormap=:algae)
fig, _ , plt = plot(Λ, jump(n_Λ(uh)))

Because of the same error:

can't splice MakieCore.ShadingAlgorithm into an OpenGL shader. Make sure all fields are of a concrete type and isbits(FieldType)-->true

Once I have this issue fixed, the tests should pass.

@hammy4815
Copy link
Contributor Author

There seems to be some sort of bug specifically with how the shaders for GLMakie are being called, and strangely this only happens for SkeletonTriangulations and BoundaryTriangulations. The bug does not exist for CairoMakie however, so the tests work now with CairoMakie...but it's worth noting this issue for GLMakie.

@hammy4815
Copy link
Contributor Author

@fverdugo ,

Do you have any thoughts regarding the GLMakie vs CairoMakie situation? Otherwise, could we see if it passes the tests this time?

@fverdugo
Copy link
Member

fverdugo commented Apr 2, 2024

Hi @hammy4815,

Thanks for your new attempt!

Since you never contributed to the project I need to manually accept the execution of the tests. Could you please do some dummy change in the repo in a separate PR? This will provably speed up the debugging of this issue.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.66%. Comparing base (0df6796) to head (5673b55).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
- Coverage   93.57%   92.66%   -0.92%     
==========================================
  Files           3        3              
  Lines         218      218              
==========================================
- Hits          204      202       -2     
- Misses         14       16       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -2,7 +2,7 @@ module TestGridapMakie

using GridapMakie
using Gridap
using GLMakie
using CairoMakie
Copy link
Member

Choose a reason for hiding this comment

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

I see that we are not testing with GLMakie anymore.

Do the tests also pass with GLMakie witht he new compat entries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests fail with GLMakie due to some backend shader issue. It works on older versions of GLMakie, and the current versions of CairoMakie, but not the newest version of GLMakie. I will try and file a minimal reproducing example with the Makie team.

@hammy4815 hammy4815 mentioned this pull request Apr 3, 2024
@dnuy
Copy link

dnuy commented May 21, 2024

The tests also pass with Gridap v0.18.2, so I suggest to update the Gridap version in the compat section.

(I also tried using the latest Makie but that does not work.)

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

5 participants