Basic script to compare two BP files given a tolerance #4308

anagainaru · 2024-08-15T13:57:09Z

Based on a request from GX developers, this is the first version of a python script to compare the information in two BP files (given a certain tolerance). The functionality is kept similar to https://gitlab.com/remikz/nccmp

Current differences detected:

Metadata (number of steps, number of variables, type and dimension of each variable)
Stats (min/max with a tolerance) - only if the --stats option is used
Values (values on each position with a tolerance)

The tolerance can be absolute value or percentage.

Example metadata differences:

$ python bpcmp.py BPFileStepsWriteRead.bp BPFileStepsWriteReadKokkos.bp
COMPARE : FILE : BPFileStepsWriteRead.bp : FILE : BPFileStepsWriteReadKokkos.bp
DIFFER : NUM_STEPS : 10 <> 1 : BPFileStepsWriteRead.bp : BPFileStepsWriteReadKokkos.bp
DIFFER : AVAIL_VARS : ['bpStep'] <> [] : BPFileStepsWriteRead.bp : BPFileStepsWriteReadKokkos.bp
DIFFER : VARIABLE : bpFloats : NUM_STEPS : 10 <> 1 : BPFileStepsWriteRead.bp : BPFileStepsWriteReadKokkos.bp
DIFFER : VARIABLE : bpFloats : SHAPE : 60000 <> 4, 3 : BPFileStepsWriteRead.bp : BPFileStepsWriteReadKokkos.bp

DIFFER : VARIABLE : bpType : TYPE :  uint32_t <> float : test1.bp : test2.bp
DIFFER : VARIABLE : bpSV : SINGLE_VALUE :  true <> false : test1.bp : test2.bp

Example min/max differences:

DIFFER : VARIABLE : bpFloats : MIN : 0 <> 1 : test1.bp : test2.bp
DIFFER : VARIABLE : bpFloats : MAX : 90 <> 60089 : BPFileStepsWriteRead.bp : BPFileStepsWriteReadHip.bp

Without verbose the script only plots a summary of the value difference

DIFFER : VARIABLE : bpFloats : VALUES_SUMMARY : 599990 : BPFileStepsWriteRead.bp : BPFileStepsWriteReadHip.bp

Using --verbose the script will plot every value difference

DIFFER : VARIABLE : bpFloats : STEP : 0 : POSITION : (5,0) : VALUE : 0.0 <> 5.0 : test1.bp : test2.bp
DIFFER : VARIABLE : bpFloats : STEP : 1 : POSITION : (6,10) : VALUE : 1.0 <> 6.0 : test1.bp : test2.bp

Things that still need to be added:

Parallel execution (add MPI support)
Specific variable inclusion or exclusion.
Print data difference statistics (count, sum, absolute sum, min, max, range, mean, stdev) for numeric variables and compound fields.
Per block comparison of min/max
Do not read the entire values for each step to compare them. Comparing per blocks might allow multi-gigabyte files to only consume several megabytes of memory.
Statistics about the difference between the values (min/max/avg/std)

source/utils/bpcmp/bpcmp.py

anagainaru · 2024-08-20T19:53:34Z

@vicentebolea could you help me with these errors?

On macos-14 where numpy is found:

File "/Users/runner/work/ADIOS2/ADIOS2/macos-14-xcode15_4-static-serial/bin/bpcmp.py", line 1, in <module>
2024-08-20T19:42:21.3772090Z 617:     import numpy as np
2024-08-20T19:42:21.3772620Z 617: ModuleNotFoundError: No module named 'numpy'
2024-08-20T19:42:21.3773080Z 614: Traceback (most recent call last):
2024-08-20T19:42:21.3773760Z 614:   File "/Users/runner/work/ADIOS2/ADIOS2/macos-14-xcode15_4-static-serial/bin/bpcmp.py", line 1, in <module>
2024-08-20T19:42:21.3774350Z 614:     import numpy as np
2024-08-20T19:42:21.3774710Z 614: ModuleNotFoundError: No module named 'numpy'
2024-08-20T19:42:21.3794100Z 617: CMake Error at /Users/runner/work/ADIOS2/ADIOS2/macos-14-xcode15_4-static-serial/Debug/bpcmp.cmake:18 (message):
2024-08-20T19:42:21.3796980Z 617:   result: 1
2024-08-20T19:42:21.3797760Z 617: 
2024-08-20T19:42:21.3798330Z 617: 
2024-08-20T19:42:21.3799920Z  10/640 Test #617: Utils.BPcmp.Verbose.Dump ................................................................................***Failed    0.11 sec
2024-08-20T19:42:21.3803270Z         Start 617: Utils.BPcmp.Verbose.Dump

And on Ubuntu 20-04

2024-08-20T19:45:50.2123317Z 899: Working Directory: /__w/ADIOS2/ADIOS2/ubuntu20.04-clang6-serial/testing/utils/bpcmp/Utils.BPcmp
2024-08-20T19:45:50.2124278Z 899: Test timeout computed to be: 120
2024-08-20T19:45:50.5971137Z 899: Traceback (most recent call last):
2024-08-20T19:45:50.5973048Z 899:   File "/__w/ADIOS2/ADIOS2/ubuntu20.04-clang6-serial/bin/bpcmp.py", line 4, in <module>
2024-08-20T19:45:50.5973989Z 899:     from adios2 import Stream
2024-08-20T19:45:50.5974647Z 899: ModuleNotFoundError: No module named 'adios2'
2024-08-20T19:45:50.6133487Z 899: CMake Error at /__w/ADIOS2/ADIOS2/ubuntu20.04-clang6-serial/Debug/bpcmp.cmake:18 (message):
2024-08-20T19:45:50.6134771Z 899:   result: 1
2024-08-20T19:45:50.6135264Z 899: 
2024-08-20T19:45:50.6135791Z 899: 
2024-08-20T19:45:50.6143107Z  13/933 Test #899: Utils.BPcmp.Stats.Dump ..................................................................................***Failed    0.40 sec
2024-08-20T19:45:50.6144568Z         Start 899: Utils.BPcmp.Stats.Dump

vicentebolea · 2024-08-20T19:56:52Z

@anagainaru I recommend you to use this add_test wrapper:

ADIOS2/cmake/ADIOSFunctions.cmake

Line 72 in 4026559

function(python_add_test)

The reason is that it does all the fixes so it uses the correct python version and pythonmodule var

anagainaru · 2024-08-20T20:35:15Z

Thanks ! Can I use python_add_test to store the output of the script into a file to validate it's contents?

anagainaru · 2024-08-20T21:14:18Z

@vicentebolea I don't seem to know how to use the python test you indicated. In my case, my testing is not a python file, it's a pipeline (similar to bpls but it has a python script in the middle).
The pipeline contains a cpp file (that generates the input file for the python script), the bpcmp python script (that generates an output that is redirected to a file) and a diff (between the generated file and a redefined one).

Any advice helps, it's not straight forward to know how to take some parts of the python_add_test and add them to my test.

vicentebolea · 2024-08-20T21:24:50Z

I See, you need to extract from the python_add_test the parts that setup the needed env variables for finding adios2 python package in the build tree and also the correct python interpreter (as you already done).

  if(UNIX)
    set_property(TEST ${ARGS_NAME} PROPERTY
      ENVIRONMENT
        "PYTHONPATH=${ADIOS2_BINARY_DIR}/${CMAKE_INSTALL_PYTHONDIR}:$ENV{PYTHONPATH}"
        "PYTHONUSERBASE=${CMAKE_BINARY_DIR}"
    )
  else()
    set_property(TEST ${ARGS_NAME} PROPERTY
      ENVIRONMENT
        "PYTHONPATH=${ADIOS2_BINARY_DIR}/${CMAKE_INSTALL_PYTHONDIR};$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_py>>;$ENV{PYTHONPATH}"
        "PATH=$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_py>>;$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_core>>;$ENV{PATH}"
        "PYTHONUSERBASE=${CMAKE_BINARY_DIR}"
    )
  endif()

anagainaru · 2024-08-21T16:55:17Z

Setting the env for python did not work, @vicentebolea any other ideas?

testing/utils/bpcmp/CMakeLists.txt

vicentebolea · 2024-08-21T21:25:40Z

You can set env variables for a command

…

On Wed, Aug 21, 2024 at 5:12 PM Ana Gainaru ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In testing/utils/bpcmp/CMakeLists.txt <#4308 (comment)>: > + ENVIRONMENT + "PYTHONPATH=${ADIOS2_BINARY_DIR}/${CMAKE_INSTALL_PYTHONDIR};$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_py>>;$ENV{PYTHONPATH}" + "PATH=$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_py>>;$<SHELL_PATH:$<TARGET_FILE_DIR:adios2_core>>;$ENV{PATH}" + "PYTHONUSERBASE=${CMAKE_BINARY_DIR}" Actually that is not going to work, because in the bpcmp.cmake.gen I am executing a command, not setting a test, and you do set_property on a test. — Reply to this email directly, view it on GitHub <#4308 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFOFRJLEVG3NRTFGFKQNTZST7C5AVCNFSM6AAAAABMSIEF5KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENJRHE2TGMBQG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- [image: logo shows large letter K with the word Kitware below it] <http://www.kitware.com> Vicente Bolea, MSc. *Senior R&D Engineer*

anagainaru · 2024-08-21T21:29:15Z

Using set() would be enough?

Something like:

set(ENV{PYTHONPATH} "All/the/paths/in/your/wrapper")

I've never set these before.

vicentebolea · 2024-08-21T21:31:26Z

Yes, it should work

…

On Wed, Aug 21, 2024 at 5:29 PM Ana Gainaru ***@***.***> wrote: Using set() would be enough? Something like: set(ENV{PYTHONPATH} "All/the/paths/in/your/wrapper") I've never set these before. — Reply to this email directly, view it on GitHub <#4308 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHFOFWSARG3X535QQOJKILZSUBEDAVCNFSM6AAAAABMSIEF5KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBTGA2DINZWGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- [image: logo shows large letter K with the word Kitware below it] <http://www.kitware.com> Vicente Bolea, MSc. *Senior R&D Engineer*

Co-authored-by: Norbert Podhorszki <pnorbert@ornl.gov>

Co-authored-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>

anagainaru · 2024-08-23T16:33:39Z

@vicentebolea the tests pass ! Thanks for the help. Could you please see if this looks good to you?

vicentebolea

Looks good, a few suggestions. I will review the python script tomorrow. Good work @anagainaru 🚀

source/utils/CMakeLists.txt

source/utils/bpcmp/CMakeLists.txt

source/utils/bpcmp/bpcmp.cmake.gen.in

testing/utils/bpcmp/CMakeLists.txt

anagainaru · 2024-08-30T17:54:30Z

@vicentebolea thanks for the suggestions. I pushed some of the changes you mentioned.

The other changes that I did not push do make sense, but I would prefer leaving this PR consistent with the other utility scripts (the functionality and build system is mimicking bpls -- namely using PROJECT_BINARY_DIR, not using quotes for paths, not using syntax=yaml, not using the maybe convention). I think all these changes need to be done to all the scripts in source/utils not just in bpcmp.

source/utils/bpcmp/bpcmp.py

vicentebolea · 2024-08-30T22:11:36Z

@vicentebolea thanks for the suggestions. I pushed some of the changes you mentioned.

The other changes that I did not push do make sense, but I would prefer leaving this PR consistent with the other utility scripts (the functionality and build system is mimicking bpls -- namely using PROJECT_BINARY_DIR, not using quotes for paths, not using syntax=yaml, not using the maybe convention). I think all these changes need to be done to all the scripts in source/utils not just in bpcmp.

You are right that this would break consistency which is very important, if you have extra cycles it would be cool to have the other utils things "Corrected" otherwise I am to strong about it. As for the maybe_ convention is through out the adios2 cmake codebase. It is a good way to point out, hey this is maybe not assigned.

vicentebolea

Few code change request in the Python code. Looks goods, feel free to take the suggestions

source/utils/bpcmp/bpcmp.py

vicentebolea · 2024-08-30T22:15:56Z

On the funny side of things I was reading this article https://www.chiark.greenend.org.uk/~sgtatham/quasiblog/code-review-antipatterns/ . I believe that I might have been guilty of some of those antipatterns haha. Ill take those lessons learned in my next reviews 👼

Co-authored-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>

anagainaru requested a review from pnorbert August 15, 2024 13:57

pnorbert reviewed Aug 15, 2024

View reviewed changes

source/utils/bpcmp/bpcmp.py Outdated Show resolved Hide resolved

anagainaru force-pushed the bpcmp branch 2 times, most recently from 804d566 to 94f20e2 Compare August 15, 2024 14:37

anagainaru requested a review from vicentebolea as a code owner August 19, 2024 20:46

vicentebolea requested review from pnorbert and removed request for vicentebolea August 19, 2024 21:02

anagainaru force-pushed the bpcmp branch from 0d7ea56 to 93c4a3f Compare August 20, 2024 19:36

anagainaru force-pushed the bpcmp branch from 43870f3 to 653ac1d Compare August 20, 2024 23:08

vicentebolea reviewed Aug 21, 2024

View reviewed changes

testing/utils/bpcmp/CMakeLists.txt Show resolved Hide resolved

anagainaru force-pushed the bpcmp branch 3 times, most recently from a3859c8 to decbe80 Compare August 22, 2024 19:59

anagainaru and others added 5 commits August 23, 2024 11:33

Basic script to compare two BP files given a tolerance

f9ce124

Code to create testing BP files for the bpcmp script

9d5553d

Create pipeline testing for bpcmp

4a8e9b9

Co-authored-by: Norbert Podhorszki <pnorbert@ornl.gov>

Removing randomness in the bpcmp test

f953ca5

Adding the python environment and interpreter for the bpcmp testing

4844206

Co-authored-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>

anagainaru force-pushed the bpcmp branch from decbe80 to 4844206 Compare August 23, 2024 15:33

anagainaru mentioned this pull request Aug 27, 2024

Implement bpcmp for comparing two bp files anagainaru/ADIOS2-addons#177

Closed

vicentebolea requested changes Aug 29, 2024

View reviewed changes

anagainaru force-pushed the bpcmp branch from 925c61e to cd84bad Compare August 30, 2024 19:59

vicentebolea reviewed Aug 30, 2024

View reviewed changes

source/utils/bpcmp/bpcmp.py Outdated Show resolved Hide resolved

vicentebolea requested changes Aug 30, 2024

View reviewed changes

source/utils/bpcmp/bpcmp.py Show resolved Hide resolved

source/utils/bpcmp/bpcmp.py Outdated Show resolved Hide resolved

source/utils/bpcmp/bpcmp.py Outdated Show resolved Hide resolved

Updates to the build system for bpcmp

37689f3

anagainaru force-pushed the bpcmp branch 2 times, most recently from eb71e41 to 8267f27 Compare August 31, 2024 01:57

Changes to the python script

ed2b352

Co-authored-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>

anagainaru force-pushed the bpcmp branch from 8267f27 to ed2b352 Compare August 31, 2024 02:26

vicentebolea approved these changes Aug 31, 2024

View reviewed changes

anagainaru merged commit 8dd2aef into ornladios:master Aug 31, 2024
38 checks passed

anagainaru deleted the bpcmp branch August 31, 2024 16:15

anagainaru mentioned this pull request Aug 31, 2024

Update the util build system based on suggestions in #4308 #4333

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic script to compare two BP files given a tolerance #4308

Basic script to compare two BP files given a tolerance #4308

anagainaru commented Aug 15, 2024 •

edited

Loading

anagainaru commented Aug 20, 2024

vicentebolea commented Aug 20, 2024

anagainaru commented Aug 20, 2024

anagainaru commented Aug 20, 2024

vicentebolea commented Aug 20, 2024

anagainaru commented Aug 21, 2024

vicentebolea commented Aug 21, 2024 via email

anagainaru commented Aug 21, 2024

vicentebolea commented Aug 21, 2024 via email

anagainaru commented Aug 23, 2024

vicentebolea left a comment

anagainaru commented Aug 30, 2024

vicentebolea commented Aug 30, 2024

vicentebolea left a comment

vicentebolea commented Aug 30, 2024

Basic script to compare two BP files given a tolerance #4308

Basic script to compare two BP files given a tolerance #4308

Conversation

anagainaru commented Aug 15, 2024 • edited Loading

anagainaru commented Aug 20, 2024

vicentebolea commented Aug 20, 2024

anagainaru commented Aug 20, 2024

anagainaru commented Aug 20, 2024

vicentebolea commented Aug 20, 2024

anagainaru commented Aug 21, 2024

vicentebolea commented Aug 21, 2024 via email

anagainaru commented Aug 21, 2024

vicentebolea commented Aug 21, 2024 via email

anagainaru commented Aug 23, 2024

vicentebolea left a comment

Choose a reason for hiding this comment

anagainaru commented Aug 30, 2024

vicentebolea commented Aug 30, 2024

vicentebolea left a comment

Choose a reason for hiding this comment

vicentebolea commented Aug 30, 2024

anagainaru commented Aug 15, 2024 •

edited

Loading