Out of place ATS #2964

cssherman · 2024-01-29T18:16:08Z

This PR moves ats files from the integratedTests repo into the main GEOS repo. The new test baselines will have a 1:1 structure to the inputFiles directory. We are planning to address changes to baseline storage in a follow-up PR.

geosPythonPackages PR:
GEOS-DEV/geosPythonPackages#6

integratedTests PR:
GEOS-DEV/integratedTests#88

host-configs/LLNL/quartz-base.cmake

cssherman · 2024-01-29T18:19:20Z

host-configs/LLNL/quartz-base.cmake

+set(ATS_BASELINE_DIR "/p/lustre2/${USER}/integratedTests/baselines"  CACHE PATH "")
+
+# Temporary argument for python module change testing
+set(GEOS_PYTHON_PACKAGES_BRANCH "feature/sherman/outOfPlaceATS" CACHE STRING "" FORCE)


This is a cmake variable that can override the branch of geosPythonPackages during the build. We'll want to have a procedure in place for handling this when we make potentially breaking changes to the CI

makes sense. We'll need that cmake target to reinstall the packages though.

Is it also working if we put and hash?

That shouldn't be hard to hard to do

@TotoGaz - This is forwarded to this git clone arg, so a hash should work: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---branchltnamegt

codecov · 2024-01-29T18:55:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3613109) 52.21% compared to head (1d34b55) 52.21%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2964   +/-   ##
========================================
  Coverage    52.21%   52.21%           
========================================
  Files          973      973           
  Lines        82572    82572           
========================================
+ Hits         43112    43114    +2     
+ Misses       39460    39458    -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CusiniM

Thanks Chris! I think having the ats files in the same folder as the xml without those soft links is great. A few things we could work on:

Naming of the ats files (it's all over the place)
Enforce smoke and benchmark naming for all xmls so that we can then easily create a filter that only runs smoke tests and one that only runs benchmarks.
consolidate to a single ats file per folder?
add a field file_name to TestDeck

TotoGaz

Did you test your PR on the CI?

Also, something that can be really interesting is to mount the source directory read only --volume=${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE_MOUNT_POINT} from https://github.com/GEOS-DEV/GEOS/blob/develop/.github/workflows/build_and_test.yml#L116
Normally, we should not touch the source folder anymore so it should be OK.

@rrsettgast that could be a very nice way not to bother about cleaning after the build: when the docker container gets removed with the --rm option, then we're good.

TotoGaz · 2024-01-29T22:00:32Z

inputFiles/compositionalMultiphaseFlow/benchmarks/SPE10/SPE10.ats

@@ -0,0 +1,19 @@
+import geos_ats


So are you adding new tests? Or are you basically "removing" the links with the file they are pointing to?

I'm just about ready to start testing on the CI. The last steps are to figure out how we want to fetch the baselines and the default path for things on the CI.

I'm not adding/removing any tests here. Essentially, the script that I wrote did the following:

Find any .ats files in the integrated tests repo

Find any corresponding .xml file links in the same repo, resolving their absolute paths

For each set of (absolute) test directories, create a new test file with correct xml names (or a flag to indicate that a file isn't present in the target directory)

This was followed by some manual cleanup of invalid tests.

We 100% should go through and further curate the test files

TotoGaz · 2024-01-29T22:01:16Z

inputFiles/efemFractureMechanics/embeddedFractures_mechanics.ats

+        check_step=1,
+        curvecheck_params=CurveCheckParameters(**curvecheck_params)),
+    TestDeck(name="SneddonRotated_smoke",
+             description='Sneddon with inclined fracture',
+             partitions=((1, 1, 1), (2, 2, 1)),
+             restart_step=0,
+             check_step=1,
+             restartcheck_params=RestartcheckParameters(**restartcheck_params))
+]


Maybe we can use yapf on ats files as well?

Yeah, already done. Something like this works in the inputFiles directory: python -m yapf -i $(find -name "*.ats")

inputFiles/main.ats

TotoGaz · 2024-01-29T22:05:17Z

host-configs/LLNL/quartz-base.cmake

+set(ATS_BASELINE_DIR "/p/lustre2/${USER}/integratedTests/baselines"  CACHE PATH "")
+
+# Temporary argument for python module change testing
+set(GEOS_PYTHON_PACKAGES_BRANCH "feature/sherman/outOfPlaceATS" CACHE STRING "" FORCE)


Is it also working if we put and hash?

src/CMakeLists.txt

TotoGaz · 2024-01-29T22:07:05Z

src/CMakeLists.txt

+    set( integrated_tests_python_sources )
+    file( GLOB_RECURSE integrated_tests_python_sources "${CMAKE_SOURCE_DIR}/../inputFiles/*.py" )
+     set( integrated_tests_ats_sources )
+    file( GLOB_RECURSE integrated_tests_ats_sources "${CMAKE_SOURCE_DIR}/../inputFiles/*.ats" )


TotoGaz · 2024-01-29T22:16:37Z

src/CMakeLists.txt

+       )
+
+    add_custom_command( OUTPUT ${ATS_SCRIPT}
+                        COMMAND ${CMAKE_BINARY_DIR}/bin/setup_ats_environment ${CMAKE_SOURCE_DIR}/.. ${CMAKE_BINARY_DIR} ${ATS_WORKING_DIR} ${ATS_BASELINE_DIR} ${ATS_ARGUMENTS}


At some moment, maybe it'll be useful to have named options like --src ...

…selines

Implementing out of place ATS, moving testing docs

cssherman added type: testing Unit tests, non-regression testing, ... flag: requires rebaseline Requires rebaseline branch in integratedTests labels Jan 29, 2024

cssherman requested review from wrtobin, rrsettgast, TotoGaz and CusiniM January 29, 2024 18:16

cssherman commented Jan 29, 2024

View reviewed changes

host-configs/LLNL/quartz-base.cmake Outdated Show resolved Hide resolved

cssherman commented Jan 29, 2024

View reviewed changes

cssherman self-assigned this Jan 29, 2024

cssherman force-pushed the feature/sherman/outOfPlaceATS branch from 2aac46e to 965ca89 Compare January 29, 2024 20:38

CusiniM reviewed Jan 29, 2024

View reviewed changes

TotoGaz approved these changes Jan 29, 2024

View reviewed changes

cssherman added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Feb 2, 2024

This was referenced Feb 2, 2024

Out of Place GEOS ATS GEOS-DEV/geosPythonPackages#6

Merged

Out of Place ATS GEOS-DEV/integratedTests#88

Merged

cssherman added 13 commits February 12, 2024 17:22

Moving ats files from integratedTests to inputFiles

17ff793

Updating the input for a temporary geos python package branch name

9063a32

Fixing new ats xml names

027e268

Fixing ats source search method

204ed51

Moving cmake instructions for integratedTests

9e70a25

Applying yapf to the .ats files

b64adca

Moving integratedTests doc file to main repo

1dd5899

Setting default values for integratedTests workflow to prepare for ba…

500d76b

…selines

Resolving merge conflict

1d563da

Temporarily setting the geosPythonPackages branch for all builds

6fc8b76

Fixing curve check paths

5535f45

Fixing hf curve check script

cbbabef

Removing duplicate test

95dfdea

cssherman and others added 3 commits February 12, 2024 17:45

Resolving merge conflicts

2de97dc

fixed some ats files.

89a7440

Resolving merge conflicts

4aec705

cssherman force-pushed the feature/sherman/outOfPlaceATS branch from 21f2c51 to 4aec705 Compare February 13, 2024 01:51

cssherman added 2 commits February 12, 2024 21:41

Updating the integrated tests

8bca8c6

Updating integrated tests

1d34b55

cssherman merged commit 728848e into develop Feb 13, 2024
23 checks passed

cssherman deleted the feature/sherman/outOfPlaceATS branch February 13, 2024 19:04

ouassimkh pushed a commit that referenced this pull request Feb 16, 2024

Out of place ATS (#2964)

b3e7781

Implementing out of place ATS, moving testing docs

arng40 pushed a commit that referenced this pull request Feb 22, 2024

Out of place ATS (#2964)

8d6e35a

Implementing out of place ATS, moving testing docs

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Out of place ATS #2964

Out of place ATS #2964

cssherman commented Jan 29, 2024 •

edited

Loading

cssherman Jan 29, 2024

CusiniM Jan 29, 2024

TotoGaz Jan 29, 2024

cssherman Jan 29, 2024

cssherman Jan 30, 2024

codecov bot commented Jan 29, 2024 •

edited

Loading

CusiniM left a comment

TotoGaz left a comment

TotoGaz Jan 29, 2024

cssherman Jan 30, 2024

cssherman Jan 30, 2024

TotoGaz Jan 29, 2024

cssherman Jan 30, 2024

TotoGaz Jan 29, 2024

TotoGaz Jan 29, 2024

TotoGaz Jan 29, 2024

Out of place ATS #2964

Out of place ATS #2964

Conversation

cssherman commented Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 29, 2024 • edited Loading

Codecov Report

CusiniM left a comment

Choose a reason for hiding this comment

TotoGaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cssherman commented Jan 29, 2024 •

edited

Loading

codecov bot commented Jan 29, 2024 •

edited

Loading