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

maint: updating cicd and start_mapdl files #223

Merged
merged 28 commits into from
Nov 28, 2023
Merged

maint: updating cicd and start_mapdl files #223

merged 28 commits into from
Nov 28, 2023

Conversation

clatapie
Copy link
Collaborator

As title says.

@ansys-reviewer-bot
Copy link

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added maintenance Package and maintenance related CI/CD bug Something isn't working labels Nov 21, 2023
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #223 (58a5f13) into main (793dc9c) will increase coverage by 7.02%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #223      +/-   ##
==========================================
+ Coverage   85.61%   92.63%   +7.02%     
==========================================
  Files           2        2              
  Lines         584      584              
==========================================
+ Hits          500      541      +41     
+ Misses         84       43      -41     

@RobPasMue
Copy link
Member

RobPasMue commented Nov 22, 2023

As I mentioned in ansys/pymapdl-examples#178 it is an issue inside the container

When running in PyMAPDL (if I am not mistaken) - you run things inside the container directly and you install these libraries inside it as well. However, here, you are connecting to it. So whenever you perform the install, it is not being installed for the container AFAICT.

@germa89
Copy link
Collaborator

germa89 commented Nov 22, 2023

As I mentioned in ansys/pymapdl-examples#178 it is an issue inside the container

When running in PyMAPDL (if I am not mistaken) - you run things inside the container directly and you install these libraries inside it as well. However, here, you are connecting to it. So whenever you perform the install, it is not being installed for the container AFAICT.

The container hasn't changed in ages (9 months)... why now it is asking for a new library to run? It does not make sense...

@germa89
Copy link
Collaborator

germa89 commented Nov 22, 2023

Opting for avoiding the issue all together....

Let's use the new docker images (do not have this issue). Hence we won't be testing for versions older than v23.1

@germa89
Copy link
Collaborator

germa89 commented Nov 22, 2023

Fixed, images are being pulled and launched properly. I would apply the same changes to pymapdl-examples
https://github.com/ansys/pyansys-math/actions/runs/6956135863?pr=223

There are some tests not passing properly. I leave it up to you @clatapie

@clatapie
Copy link
Collaborator Author

The unit test test_solve_eigs_km was failing because of a change in the MAPDL output when version > 23.2

Output for version 23.2:

*****  INDEX OF DATA SETS ON RESULTS FILE  *****\n\n   SET   TIME/FREQ    LOAD STEP   SUBSTEP  CUMULATIVE\n     1  1475.1             1         1         1                  \n     2  1475.1             1         2         2                  \n     3  2018.8             1         3         3                  \n     4  2018.8             1         4         4                  \n     5  2018.8             1         5         5                  \n     6  2024.8             1         6         6                  \n     7  2024.8             1         7         7                  \n     8  2024.8             1         8         8                  \n     9  2242.2             1         9         9                  \n    10  2274.8             1        10        10

Output for version 24.1:

*****  INDEX OF DATA SETS ON RESULTS FILE  *****\n\n     SET   TIME/FREQ    LOAD STEP   SUBSTEP  CUMULATIVE\n       1  1475.1333             1         1         1                  \n       2  1475.1333             1         2         2                  \n       3  2018.8374             1         3         3                  \n       4  2018.8374             1         4         4                  \n       5  2018.8374             1         5         5                  \n       6  2024.7868             1         6         6                  \n       7  2024.7868             1         7         7                  \n       8  2024.7868             1         8         8                  \n       9  2242.2153             1         9         9                  \n      10  2274.8300             1        10        10

The number of digits after the decimal point increased from 1 (for 23.2) to 4 ( for24.1). The number were therefore no longer caught by the re.findall call.

doc/source/conf.py Outdated Show resolved Hide resolved
Co-authored-by: Roberto Pastor Muela <37798125+RobPasMue@users.noreply.github.com>
@RobPasMue
Copy link
Member

Ready to merge?

@clatapie clatapie enabled auto-merge (squash) November 28, 2023 16:57
@clatapie clatapie disabled auto-merge November 28, 2023 17:08
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
@clatapie clatapie enabled auto-merge (squash) November 28, 2023 17:09
@clatapie clatapie merged commit 20f567d into main Nov 28, 2023
@clatapie clatapie deleted the fix/cicd branch November 28, 2023 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maintenance Package and maintenance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants