Pf-keys: Constrain lists of names + bugfix #352
Conversation
6ae8eba to
3482d45
Compare
3482d45 to
ab74a05
Compare
|
Is this ready for review? Please mark MR that are not ready with DRAFT: or WIP: |
|
Marked wip, thanks.
…On Fri, Aug 6, 2021 at 4:24 PM Steven Smith ***@***.***> wrote:
Is this ready for review? Please mark issues that are not ready with
DRAFT: or WIP:
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#352 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR2FNTRJWQG5E3OJINHKK6DT3ROJJANCNFSM5BUXSAXQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email>
.
|
|
@smithsg84 @jourdain This is now ready for review. |
|
LGTM, should be ready for review now. |
|
Several python tests are failing, is this passing in your local env? https://github.com/parflow/parflow/pull/352/checks?check_run_id=3293124657 |
|
I'm looking into these tests which are failing locally on this branch, but not on master:
|
|
@DrewLazzeriKitware are the tests working locally now? |
With the change two more are passing. I'm still looking at the remaining three. |
9e12395 to
b84f914
Compare
|
There are three tests still failing on my machine, but I found that the same tests are failing in the master branch on my machine: @smithsg84 Could we trigger the CI again to see if these clear up? |
Squashed commit of the following:
commit 1a24e869dd0436fb5196b2a2a8a6fc62a36c35c4
Author: Drew Lazzeri <drew.lazzeri@kitware.com>
Date: Wed Aug 11 20:17:38 2021 -0400
fix handle empty list cases
commit 7a61919db8eb76a6b5a1aa21176d4cd4b9794543
Author: Drew Lazzeri <drew.lazzeri@kitware.com>
Date: Wed Aug 11 20:11:43 2021 -0400
Fix select should consider prefixes like _0
commit a7e208197ed79596ed977c04507006bb1edb11ac
Author: Drew Lazzeri <drew.lazzeri@kitware.com>
Date: Wed Aug 11 20:08:44 2021 -0400
fix ListHandler comparisons
b84f914 to
f73f408
Compare
|
I've forced pushed to remove some auto-formatting that made diffs unreadable - the tests pass on the unformatted and pushed version though. |
|
The 3_3_3 tests is running a 27 rank tests, that could be failing if your MPI prohibits oversubscribing cores, frequently you can pass extra flags to MPI to enable this (-oversubscribe for OpenMPI) The FindMPI in CMAKE can set extra MPI flags. https://cmake.org/cmake/help/latest/module/FindMPI.html The oversubscribing doesn't run to poorly for simple tests. You may also need to turn off busy waiting: Not sure why the other test would be failing. |
1) Update some documentation for contributing to pf-keys
2) Fix a bugs found in pf-keys where some keys failed to set when unless set in a particular order
3) Add constraint for lists of names
This change lets us express that one list of names should be a subset of another list of names
Constraint Example
Values for
PhaseSources.{phase_name}.GeomNamesshould be a subset of values from eitherGeomInput.{geom_input_name}.GeomNamesorGeomInput.{geom_input_name}.GeomName. Setting the domain toEnumDomainlike so expresses that constraint. A more detailed example can be seen in this test case.