GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add test matrix against operating environments by murrayrm · Pull Request #821 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Dec 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This PR adds a GitHub action to run unit tests against a (large) matrix of different environments, including operating system, BLAS variant, and installation/build method (pip, conda), copied over from slycot. This is in response to issue #820, where an error was triggered under Ubuntu using the generic BLAS library for NumPy 1.24.0.

The matrix of tests is quite time consuming (about 45 minutes), and so the action is only trigged on pull requests (so if you are using GitHub actions in your own forks, they won't be affected).

In the initial PR, the errors from issue #820 are present (so the tests will fail). In a subsequent push to this PR, I added a "fix" by doing an xfail on that specific test case (and demonstrating one way this set of test might be used).

Also: when a failure occurs in the flatsys test case, information is printed out about the configuration to help in identifying the cases that are failing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
murrayrm added 3 commits December 22, 2022 22:11
@murrayrm
add OS/BLAS pip-based test matrix as GitHub workflow
9d72d79
@murrayrm
add OS/BLAS conda-based test matrix as GitHub workflow
1201202
@murrayrm
triger on pull_request
f477c13
@murrayrm
murrayrm force-pushed the test_matrix-22Dec2022 branch from f246732 to f477c13 Compare December 24, 2022 04:00
@murrayrm murrayrm linked an issue Dec 24, 2022 that may be closed by this pull request
Flaky test failure in TestFlatSys with current slycot #820
Closed
@coveralls

coveralls commented Dec 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

Coverage: 94.803%. Remained the same when pulling 4d6a6bf on murrayrm:test_matrix-22Dec2022 into 56f227f on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@murrayrm
add flatsys xfails for platform=ubunto, BLAS=Generic, numpy=1.24.0
b8c4e5c
@murrayrm

murrayrm commented Dec 24, 2022

Copy link
Copy Markdown
Member Author

Added checks + xfail for Ubuntu with generic BLAs with numpy 1.24.0, in response to issue #820. Near as I can tell, this is some sort of edge case that gets triggered under this specific set of circumstances. Not sure this is the right way to handle it, but given that all of the other environments work out, I don't think it is a bug in solve_flat_ocp.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bnavigator

bnavigator commented Dec 24, 2022

Copy link
Copy Markdown
Contributor

python-control is pure Python and should not depend on the environment. Problems in the environment usually show up when the consumed libraries change, not during python-control PRs.

We already go through that test matrix in the Slycot CI and run the python-control test suite there. Not sure if we really should duplicate the resource consumption here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@murrayrm
change trigger to workflow_dispatch or PR against relevant files
4d6a6bf
@murrayrm

murrayrm commented Dec 24, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

I agree this shouldn't be needed @bnavigator, but the failure pointed out in issue #820 is an example of where python-control does depend on the operating environment, because there is some sort of (non-slycot) interaction with something in the build environment (and only under Ubuntu, NumPy 1.24, generic BLAS).

I've changed the trigger to workflow_dispatch, which allows this to be manually triggered on the main branch, plus on PRs that affect files specifically associated with this workflow. That will get rid of the resource consumption except in the rather exceptional cases where we need it. (I also imagine that we might customize the test matrix here over time, so it could diverge from slycot.)

My current plan is to add a step in the release Instructions suggesting running this workflow before a release, just to flag anything where python-control depends on the details of the OS, BLAS version, or installation method.

bnavigator reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@murrayrm
murrayrm merged commit f15fc0f into python-control:main Dec 24, 2022
@murrayrm murrayrm mentioned this pull request Dec 25, 2022
update workflow to pass jobname to python-control pytest python-control/Slycot#190
Merged
@murrayrm murrayrm added this to the 0.9.3 milestone Dec 29, 2022
@bnavigator bnavigator mentioned this pull request Oct 18, 2025
Use pytest markers for slycot, cvxopt, and pandas tests #1182
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.3

Development

Successfully merging this pull request may close these issues.

Flaky test failure in TestFlatSys with current slycot

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@murrayrm @coveralls @bnavigator
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.