ansys / pymapdl

Pythonic interface to MAPDL
https://mapdl.docs.pyansys.com
MIT License
433 stars 121 forks source link

Improve plotting quality #2301

Open germa89 opened 1 year ago

germa89 commented 1 year ago

Checking the pytest-pyvista extension, I have realised that the plotting quality is not great (at least on the CICD).

Plots for demonstration:

On the left, the ones generated in my laptop (MacOS X, Python 3.10, MAPDL v22.2). On the right, the ones generated inside a MAPDL Ubuntun container with (Ubuntu, Python 3.10, MAPDL v22.2).

image image image image
germa89 commented 1 year ago

Pinging @ansys/pyansys-core team for feedback. Especially @AlejandroFernandezLuces

AlejandroFernandezLuces commented 1 year ago

This problem, as well as all of the problems we've seen with PyVista screenshots in PyMAPDL lead me to think that there is a problem with how PyVista is getting the screenshots when the plotter is not in visual mode.

I've already reviewed the scraper for PR #2046, which was potentially where problems like this could arise, and everything is looking good there at the moment. The only problematic part left is the PyVista plotter. PyMAPDL is using PyVista fine, so there shouldn't be anything wrong here regarding this.

We will need assistance from PyVista people to fix these errors. As soon as we get in touch with them, I'll arise this issue with them.

germa89 commented 1 year ago

@AlejandroFernandezLuces is there a way to bypass all the fails in pytest??

AlejandroFernandezLuces commented 1 year ago

You want to temporary mark those test to be skipped because they will fail, while it doesn't get fixed right? You can skip specific tests if you need it with this decorator:

https://docs.pytest.org/en/7.1.x/how-to/skipping.html#skipping-test-functions

Also, you can skip tests based in a condition too, for example:

https://github.com/pyvista/pyvista/blob/9c53f64238c2adc097c0f145dd0facee19d3ff3d/tests/plotting/test_actor.py#L10-L12

Something like this should work:


skip_pyvista = pytest.mark.skip(reason="PyVista is not getting the right screenshots")

@skip_pyvista
def test_yourtest():
   ...
germa89 commented 1 year ago

@AlejandroFernandezLuces

I recently implemented #2359 which basically recache the images in the PR when they don't match the ones in the repo. This is quite useful given how unstable is pyvista backend.

You can can see an example here: https://github.com/ansys/pymapdl/pull/2420/commits/a1c5b7414e6b7ff0fceaae3b6afa25b0e60cb3cb

I hope this example can help us to identify issues with the backend.

germa89 commented 11 months ago

@AlejandroFernandezLuces is it worthy to ping Pyvista mantainers? (Bane, Alex, Tetsuo)

AlejandroFernandezLuces commented 11 months ago

It might be worth it. Let me double check after the hackathon this issue and I'll let you know

AlejandroFernandezLuces commented 5 months ago

Update on this: