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

ENH: add error filtering; optionally reuse SubprocessTao in the test suite #95

Merged
merged 9 commits into from
Sep 17, 2024

Conversation

ken-lauer
Copy link
Collaborator

@ken-lauer ken-lauer commented Sep 6, 2024

Changes

Closes #85
Closes #96
Closes #97 (context manager portion; not weakref)

  • Reduces build/test workflow on CI from 20 minutes to 10 minutes (roughly 8 minutes for the test suite)

    • Locally, this about 4 minutes for just the test suite
  • Added filter_tao_messages() and filter_tao_messages_context() to selectively filter Tao output messages that originate from certain Fortran functions

    • filter_tao_messages applies to all Tao calls throughout the life of the program:
      filter_tao_messages(functions=["tao_find_plot"])
      tao = Tao(init_file="...")
    • filter_tao_messages_context is a context manager that can be used for specific sections of code:
      with filter_tao_messages_context(functions=["tao_find_plot"]):
          tao = Tao(init_file="...")
  • Message filtering can be done with a bit of granularity:

    • Message capture can be done globally (functions, a list of Tao Fortran function names)
    • Message capture can be done on a per-level basis (by_level, a dictionary of severity - e.g., "ERROR" - to a list of function names)
    • Message capture can be done on a per-command basis (by_command, a dictionary of command - e.g., "set" - to a list of function names)
  • Test suite: reuse SubprocessTao in the test suite

    • Disabled if environment variable TAO_REUSE_SUBPROCESS=n, pytest-xdist is detected, or GitHub Actions runner debug mode is on
    • Test suite SubprocessTao utilizes capture() to selectively exclude the problematic tao_find_plots messages only during init
  • SubprocessTao itself is now a context manager, for easier resource management

  • Add tao.last_output to get the last text output retrieved from Tao

TODO

Background

  • David noted that this PR took too long for his development style: CI: run the pytao test suite with every bmad commit bmad-ecosystem#1174
  • The test suite is slow in the main branch:
    • In small part due to driving_terms taking a long time to initialize (~1 minute)
    • Primarily due to our tests of SubprocessTao - which spins up a new, isolated subprocess with Tao (~10 minutes)
      • It's good practice to keep tests isolated from one another (i.e., one test's results shouldn't interfere with another's)
      • More practically it's because reinitialization of Tao can be buggy

With this PR, locally I can make the entire test suite run in 3:30 (vs about 15 minutes) on my laptop reusing a single subprocess. However, Tao itself fails to reinitialize in 13 tests, so we will need to address those test cases.

Fixing the failing test cases indicated that it was time to address #96 and #85 at the same time.

  • The reason SubprocessTao was succeeding where Tao was failing was due to a long-standing bug Tao() initialization ignores ERROR in output on first init (but not on reinitialization) #96 where initialization vs reinitialization error handling was inconsistent:
    • The first initialization of ctypes Tao would call tao_c_init_tao and only error if Tao itself reported an error
    • The second (and beyond) initializations would no longer rely on the above, instead checking for ERROR and similar in the output text lines
    • This meant that "ERROR" may have been present in the first initialization but was never propagated to the user

Now, the ERL example startup file includes plotting commands:

  • These plotting commands assume Tao's plotting mechanism is being used and not -external_plotting
  • These commands output errors which pytao picks up and mistakes as fatal
  • Thus, addressing Add ability to filter certain error messages #85 would then solve the case where we need to ignore certain messages at certain times

@ken-lauer ken-lauer changed the title TST: optionally reuse SubprocessTao in the test suite to make it run faster WIP/TST: optionally reuse SubprocessTao in the test suite; add error filtering Sep 10, 2024
@ken-lauer ken-lauer changed the title WIP/TST: optionally reuse SubprocessTao in the test suite; add error filtering WIP/ENH: add error filtering; optionally reuse SubprocessTao in the test suite Sep 10, 2024
@ken-lauer ken-lauer marked this pull request as ready for review September 17, 2024 18:06
@ken-lauer ken-lauer changed the title WIP/ENH: add error filtering; optionally reuse SubprocessTao in the test suite ENH: add error filtering; optionally reuse SubprocessTao in the test suite Sep 17, 2024
@ChristopherMayes ChristopherMayes merged commit 0959073 into bmad-sim:master Sep 17, 2024
6 checks passed
@ken-lauer ken-lauer deleted the tst_reuse_subprocess branch September 17, 2024 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants