Add WASP submodule and set up in HIT input processing implementation

brandonlangley commented 1 year ago

refs #22015

loganharbour commented 1 year ago

I'm probably going to need to change CI quite a bit for this. There are quite a few places where we rely on not initializing any submodules at all within moose

dschwen commented 1 year ago

Neat, looks like you added a compatibility layer around WASP that fully emulates the HIT API. Could you please make sure this works with https://github.com/idaholab/moose/blob/next/framework/contrib/hit/main.cc

Also this will add some work for me to build the emscripten compiled version of the hit utility for the VSCode plugin, which is currently trivial, but will then have to use the WASP build system. So I'll probably bug you about that at some point.

brandonlangley commented 1 year ago

@permcody -

Here's the new HIT processor that integrates WASP and uses it to implement all of the lexing and parsing under the hood.

You told me the following regarding the desire to be able to switch the two HIT versions out with each other for first testing.

Will your PR add CPP directives to switch over to WASP if it exists and fallback to HIT?

We'll need something like for this first stage of testing.

Later, we'll probably want to make the build of this package part of the normal MOOSE build.

But let's not worry about that yet.

So I was careful in developing an adapter class that fulfills 100% of the current HIT public parse API and node API externally.

The WASP interpreter and node views are used to build the HIT Node tree so all of the current interfaces work seamlessly.

Either version of the HIT parser can be used and it should be transparent to any users, applications, or downstream code.

I spent some time working on a way to toggle on the new HIT when WASP is available and the old HIT to be used otherwise.

However after trying many different approaches, I didn't get a clean method completely working to automatically toggle.

Since WASP will be required for HIT and the language server, I wasn't sure a setup based on WASP availability was needed.

If it is, you would definitely know the preferred way to handle that in the MOOSE framework exponentially faster than me.

So in this initial push of the code, WASP is required and the new WASP-HIT version of the API is used to process all input.

As it currently stands, the CI must call the update_and_rebuild_wasp.sh script as a pre-req for it to build without error.

All the same methods and classes are available in the new and old HIT versions but are scoped to different namespaces.

I temporarily left the hit:: namespace for the legacy HIT and added a wasp_hit:: namespace to scope the new version.

After this work is reviewed but before merging, I can switch the new WASP-HIT version into the familiar hit:: namespace.

The WASP submodule points to a branch now because some changes still need to be merged, but it will change to master.

Also, I have a rough high-level blueprint mapped out for the initial language server hookup with Workbench that involves:

 - add `--language-server` to MooseApp calling start_server()

   - where start_server() does the following

     - wait for initialize() request on the connection

       - send initialized() response on the connection

     - wait on document_open() request on the connection

       - call get_diagnostics() with the document text

         - send diagnostics() response on the connection

       - call symbol_tree_builder() with the HIT tree

         - send document_symbols() on the connection

     - wait on document_change() request on the connection

       - call get_diagnostics() with the document text

         - send diagnostics() response on the connection

       - call symbol_tree_builder() with the HIT tree

         - send document_symbols() on the connection

   - where get_diagnostics() does the following

     - parse the input storing syntax errors

     - if pass - check unused parameters

     - if pass - check duplicate parameters

     - if pass - check required parameters

     - if pass - any similar further checks

     - catch and return all MooseErrors

   - where symbol_tree_builder() does the following

     - recursively walks the complete HIT tree

     - builds document symbol tree hierarchy

     - returns tree that satisfies the protocol

 - in the NEAMS Workbench

   - hookup `moose_app-opt --language-server`

   - identical to current hookup of `mcnp-server'

   - input diagnostics sent from MOOSE will show

   - nav tree built continually from symbol tree

Note that the API for all server-side protocol communication pieces have already been implemented and tested in WASP.

And the client-side communication API has already been added to Workbench and is being used by another application.

But adding language server diagnostics should probably wait for a different PR after getting WASP integrated on this one.

Most of my testing was done with the 3,0200+ tests within the test/ directory and fixing issues until 100% were passing.

I finally got all issues ironed out yesterday so all of these tests are passing, then last night I noticed the unit test directory.

So there is currently one failing unit test, HitTest.C, and I have not had time to cycle through diagnoses and fixes yet.

At a quick glance, it looks like the failures in this unit test are mostly just minor discrepancies in handling a few cases like:

[ FAIL ] - Tests for FailCases               --> There are slight differences in the set of characters allowed in specific scopes.

[ PASS ] - Tests for LineNumbers

[ PASS ] - Tests for PassCases

[ PASS ] - Tests for ExplodeParentless

[ FAIL ] - Tests for ParseFields             --> Field of the form 'foo = e-23' reported a value type of string rather than float.

[ FAIL ] - Tests for BraceExpressions        --> WASP throws a parse error for multi-ine brace expressions that I hadn't yet seen.

[ PASS ] - Tests for RenderParentlessSection

[ PASS ] - Tests for RenderSubsection

[ FAIL ] - Tests for RenderCases             --> Position of in-line comments and consecutive blank lines not perserved on render.

[ PASS ] - Tests for MergeTree

[ FAIL ] - Tests for Clone                   --> Boolean option that asks cloned node names to be set to absolute paths is unused

[ PASS ] - Tests for GatherParamWalker

[ PASS ] - Tests for RemoveParamWalker

[ PASS ] - Tests for RemoveEmptySectionWalker

[ PASS ] - Tests for Formatter

[ PASS ] - Tests for Formatter_sorting

[ PASS ] - Tests for unsigned_int

[ PASS ] - Tests for vector_unsigned_int

These appear to be easy fixes that I'm hoping to look into later this week, but that shouldn't hold up any initial review.

dschwen commented 1 year ago

What's going on with the empty lines between every line? First here and now your comment. :-D

dschwen commented 1 year ago

I spent some time working on a way to toggle on the new HIT when WASP is available and the old HIT to be used otherwise.

Check out this logic in the MOOSE app Blackbear (the optional submodule is Argonne's NEML library):

https://github.com/idaholab/blackbear/blob/devel/contrib/neml.mk#L1-L38

We check if a file underneat the submodule location exists (here the CMakeLists.txt file). If so we add the NEML_ENABLED preprocessor symbol to teh compile ocmmands for teh MOOSE app: https://github.com/idaholab/blackbear/blob/ccb87ba1833ebdba7192a4668a4b704d871009ec/contrib/neml.mk#L28

permcody commented 1 year ago

Thanks @brandonlangley - I'll start digging into this soon!

brandonlangley commented 1 year ago

@permcody -

Thanks, over the past week I've actually fixed all of the outstanding issues revealed by the HitTest.C unit test.

This includes fixes such as correctly rendering blank lines, chaining together consecutive string literals, etc. etc.

After tidying a few things, I'm planning to squash these changes into my current commit and force push it today.

So you might want to hold off until I get that pushed and you not have to worry about the delta between the two.

brandonlangley commented 1 year ago

@permcody -

I just pushed the fix-ups, and all of the tests should pass using WASP-HIT now including that HitTest.C unit test.

As mentioned above, currently scripts/update_and_rebuild_wasp.sh has to be called for it to build without error.

I got everything like the rendering / newline details, combining consecutive string literal, etc, to match the old HIT.

Except in that HitTest.C unit test, you'll see three comments explaining three very minor tweaks made to the test.

It seemed to me that these three specific test tweaks were probably okay - but when you see you can let me know.

And again, I did this in a way so that the exact HIT API is fulfilled by WASP and either one can be used seamlessly.

As you had mentioned that being able to swap them in and out would be a needed first step in this stage of testing.

So I did reuse a lot of the old logic from HIT routines without trying to improve things that could affect its behavior.

Improvements can be made after WASP-HIT is adopted when we don't need to make sure exact behaviors match.

But this first pass was just an attempt to replicate the old HIT behavior exactly to help make any transition smooth.

brandonlangley commented 1 year ago

@permcody -

I just rebased this on the latest changes, fixed merged conflicts, and captured new updates in wasp-hit such as

milljm commented 1 year ago

I've checked out this branch, but I can't build it:

/home/milljm/projects/moose/framework/contrib/hit/wasp_parse.h:14:10: fatal error: wasphit/HITInterpreter.h: No such file or directory
   14 | #include "wasphit/HITInterpreter.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

Also, would it be possible to do what Civet is suggesting:

BUILD_ROOT/moose/: precheck_errors c8ebf494b57f8d9c718405208a8445e3df28cff5 HEAD
TICKET_REFERNCES:     refs #22015

##########################################################################
INFO: Your patch contains a valid ticket reference.
INFO: Your patch contains no trailing whitespace.
INFO: Your patch contains no tabs.
INFO: Your patch contains no files without newlines before EOF.
INFO: Your patch contains no bad executable files.
INFO: Your patch contains no banned functions.
INFO: Your patch contains no banned keywords.
INFO: Style check disabled
INFO: Your patch contains no unicode characters.
INFO: Your patch contains no old style C++ include guards.
##########################################################################

##########################################################################
ERROR: The following files contain keywords classified or proprietary keywords:
    unit/src/HitTests.C
##########################################################################

ERROR: exiting with code 1

You can see these results following the links provided by the details link for Checks below...

EDIT: I forgot to run ./update_and_rebuild_wasp.sh :) my bad!

brandonlangley commented 1 year ago

@milljm

I've checked out this branch, but I can't build it

As mentioned above, currently scripts/update_and_rebuild_wasp.sh has to be called first for it to build without error.

This will clone WASP, build it, and install the headers and libs it to a location where the build system should find them.

So as of now, CI will also have to call that script as a pre-req for it to build without error.

Also, what exactly does this indicate:

##########################################################################
ERROR: The following files contain keywords classified or proprietary keywords:
    unit/src/HitTests.C
##########################################################################

Did I add a specific word to that unit test that is not allowed? If so, is there any way to know what that word is?

milljm commented 1 year ago

@milljm

Also, what exactly does this indicate:
##########################################################################
ERROR: The following files contain keywords classified or proprietary keywords:
  unit/src/HitTests.C
##########################################################################
Did I add a specific word to that unit test that is not allowed? If so, is there any way to know what that word is?

It just means it found the literal word "classified" or "proprietary" in the following file types: "*.[Chi]" "*.py"

milljm commented 1 year ago

FYI, we can change the package name to whatever you wish. Right now it would be: moose-wasp.

We should prefix it with something, is my only request. Otherwise we run the risk of colliding with same-named packages on other main channels (like main/, conda-forge, etc).

That said, I have manually made the package available on our public channel. If you wish to use it:

Add the public channel if you haven't already:

conda config --add channels https://conda.software.inl.gov/public

Then install the package:

mamba activate moose
mamba install moose-wasp

or, in a separate channel of your choosing:

mamba activate base
mamba create -n wasp moose-wasp
mamba activate wasp

By activating the environment, $WASP_DIR gets set. As well as the path: $WASP_DIR/bin added to your $PATH.

brandonlangley commented 1 year ago

@milljm

It just means it found the literal word "classified" or "proprietary" in the following file types: "*.[Chi]" "*.py"

Thanks, I just added a commit that removes the word "classified" from HitTests.C and updates the WASP submodule.

So now the CIVET prechecks pass, but things fail later with that error you saw when WASP wasn't installed, as expected.

/data/civet0/build/moose/framework/contrib/hit/wasp_parse.h:14:10: fatal error: wasphit/HITInterpreter.h: No such file or directory
   14 | #include "wasphit/HITInterpreter.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~

I also rebased on upstream/next to resolve a merge conflict in the .gitmodules file and force-pushed it back here.

brandonlangley commented 1 year ago

@milljm -

Is there a reason you pushed this commit here two days ago (Jan 3) reverting these previous changes that I made?

removing the word "classified" from HitTests.C
updating the WASP submodule to the latest
fixing a clang format spacing issue in Parser.C
rebasing to resolve a conflict in .gitmodules

So now, "classified" is back and the CIVET pre-test precheck is failing again with:

##########################################################################
ERROR: The following files contain keywords classified or proprietary keywords:
    unit/src/HitTests.C
##########################################################################

And after that, the CIVET clang-format precheck would fail again with:

----------------------- Start diff -----------------
diff --git a/framework/src/parser/Parser.C b/framework/src/parser/Parser.C
index d687202b36f..5415c040cd2 100644
--- a/framework/src/parser/Parser.C
+++ b/framework/src/parser/Parser.C
@@ -464,11 +464,11 @@ Parser::walkRaw(std::string /*fullpath*/, std::string /*nodepath*/, wasp_hit::No
   if (iters.first == iters.second)
   {
     _errmsg += wasp_hit::errormsg(n,
-                             "section '[",
-                             curr_identifier,
-                             "]' does not have an associated "Action".
 Common causes:
"
-                             "- you misspelled the Action/section name
"
-                             "- the app you are running does not support this Action/syntax") +
+                                  "section '[",
+                                  curr_identifier,
+                                  "]' does not have an associated "Action".
 Common causes:
"
+                                  "- you misspelled the Action/section name
"
+                                  "- the app you are running does not support this Action/syntax") +
                "
";
     return;
   }
----------------------- End diff -----------------

I'm not sure if your commit was done by accident and should be reverted?

milljm commented 1 year ago

oops, yes this should be reverted, I will do it...

brandonlangley commented 1 year ago

@milljm -

oops, yes this should be reverted, I will do it...

Thanks a lot ... also should scripts/update_and_rebuild_wasp.sh be run on the CIVET boxes to get past this build error?

/data/civet0/build/moose/framework/contrib/hit/wasp_parse.h:14:10: fatal error: wasphit/HITInterpreter.h: No such file or directory
   14 | #include "wasphit/HITInterpreter.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

@loganharbour - I'm not sure what the next step is that needs to happen to get this merged?

Does someone with more framework experience than me need to set up the fallback to old HIT if WASP is unavailable?

loganharbour commented 1 year ago

I'll evaluate where we are on this either this weekend or Monday. Need to pull out our notes from when we chatted last, but I recall our plans for making this as swappable as possible.

brandonlangley commented 1 year ago

@loganharbour -

I see there's a small one-line merge conflict now in Parser.h where it got updated on master.

I'm planning to start back on the language server work this week and will fix that conflict too.

I'll evaluate where we are on this either this weekend or Monday. Need to pull out our notes from when we chatted last, but I recall our plans for making this as swappable as possible.

Did you get a chance a couple of week ago to look into this and see what needs to get this merged?

brandonlangley commented 1 year ago

@loganharbour -

I just rebased this branch on devel and resolved the merge conflict in Parser.h

So this is ready again to be merged pending any infrastructure updates you guys make for HIT swappability.

A wasp_hit:: namespace is currently added for the new WASP-HIT with downstream code updated to use it.

All of the tests pass on my machine with this wasp_hit:: as the version of HIT that is being used.

So everything should build in CIVET and run fine if those CI boxes just get WASP installed by either:

running scripts/update_and_rebuild_wasp.sh or
using @milljm's new moose-wasp conda package

Did you happen to get a chance a few weeks back to look into what the steps need to be to get this merged?

@milljm -

Thanks a lot for making the moose-wasp conda package - I have one (hopefully small) request regarding that.

I have been testing everything until now using a WASP installed from my update_and_rebuild_wasp.sh script.

But I wanted to start testing also by using your new moose-wasp conda package, so I ran:

conda config --add channels https://conda.software.inl.gov/public
mamba activate moose
mamba install moose-wasp

and got this error:

Could not solve for environment specs
Encountered problems while solving:
  - nothing provides requested moose-wasp

The environment can't be solved, aborting the operation

I'm using a MacBook with one of the new Apple M1 chips, so I'm using Mambaforge-MacOSX-arm64.

After looking at the public conda channel, I found these conda packages available per architecture:

                      linux-64    osx-64    osx-arm64

moose                    [X]        [X]        [X]
moose-libmesh            [X]        [X]        [X]
moose-libmesh-vtk        [X]        [X]        [X]
moose-mpich              [X]        [X]        [X]
moose-peacock            [X]        [X]        [X]
moose-petsc              [X]        [X]        [X]
moose-pprof              [X]        [X]        [X]
moose-pyhit              [X]        [X]        [X]
moose-seacas             [X]        [X]        [X]
moose-test-tools         [X]        [X]        [X]
moose-tools              [X]        [X]        [X]
moose-wasp               [X]        [X]        [ ]

So moose-wasp is the one conda package available for only linux-64 and osx-64 but not osx-arm64.

Would it be much trouble to add an Apple Silicon osx-arm64 conda package moose-wasp like the others?

I would love to test it out on my M1 chip MacBook as I continue to work on language server development.

loganharbour commented 1 year ago

@brandonlangley, @milljm and I are chatting about this today. I think we've decided that the move forward here is to default to HIT if WASP isn't available, that way we don't add in a new build requirement for now to users if they're not using conda. I'm playing around with this to see if I can get it the way I like it.

milljm commented 1 year ago

I can make it available in osx-arm64. I just need to manually build it (since we don't have a Conda recipe doing this for us yet).

milljm commented 1 year ago

@brandonlangley moose-wasp should be available among the osx-arm64 channel now...

brandonlangley commented 1 year ago

@milljm -

Thanks a lot for adding the osx-arm64 version of moose-wasp to the public conda channel.

I updated wasp_DIR in moose.mk to WASP_DIR matching the environment variable case set by moose-wasp.

When I went to install the new moose-wasp conda package, it told me it needed to downgrade these packages:

Transaction

  Prefix: /Users/orl/mambaforge3/envs/moose

  Updating specs:

   - moose-wasp
   - ca-certificates
   - certifi
   - openssl

  Package                 Version  Build          Channel                                      Size
─────────────────────────────────────────────────────────────────────────────────────────────────────
  Install:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  + moose-wasp              1.0.0  build_0        conda.software.inl.gov/public/osx-arm64       3MB

  Change:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  - libpq                    15.1  hbce9e56_3     conda-forge                                      
  + libpq                    15.1  hb650857_1     conda-forge/osx-arm64                         2MB
  - moose-libmesh-vtk       9.1.0  h6b51079_17    conda.software.inl.gov/public                    
  + moose-libmesh-vtk       9.1.0  h18806a5_16    conda.software.inl.gov/public/osx-arm64      27MB
  - moose-mpich             4.0.2  build_7        conda.software.inl.gov/public                    
  + moose-mpich             4.0.2  build_6        conda.software.inl.gov/public/osx-arm64       7kB
  - moose-petsc            3.16.6  build_6        conda.software.inl.gov/public                    
  + moose-petsc            3.16.6  build_5        conda.software.inl.gov/public/osx-arm64      25MB

  Downgrade:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  - krb5                   1.20.1  h127bd45_0     conda-forge                                      
  + krb5                   1.19.3  hf9b2bbe_0     conda-forge/osx-arm64                         1MB
  - libcurl                7.87.0  hbe9bab4_0     conda-forge                                      
  + libcurl                7.86.0  hd538317_1     conda-forge/osx-arm64                       332kB
  - libnghttp2             1.51.0  hd184df1_0     conda-forge                                      
  + libnghttp2             1.47.0  h232270b_1     conda-forge/osx-arm64                       835kB
  - libtool                 2.4.7  hb7217d7_0     conda-forge                                      
  + libtool                 2.4.6  hbdafb3b_1008  conda-forge/osx-arm64                       534kB
  - moose-libmesh      2023.01.24  build_0        conda.software.inl.gov/public                    
  + moose-libmesh      2022.11.07  build_2        conda.software.inl.gov/public/osx-arm64      36MB

  Summary:

  Install: 1 packages
  Change: 4 packages
  Downgrade: 5 packages

  Total download: 95MB

─────────────────────────────────────────────────────────────────────────────────────────────────────

Confirm changes: [Y/n] Y

After continuing to install moose-wasp and subsequently downgrading the other packages, my build had this error:

In file included from /Users/orl/projects/moose/framework/build/unity_src/meshgenerators_Unity.C:65:
/Users/orl/projects/moose/framework/src/meshgenerators/SymmetryTransformGenerator.C:90:28: error: no type named 'orient_elements' in namespace 'libMesh::MeshTools::Modification'
  MeshTools::Modification::orient_elements(*mesh);
  ~~~~~~~~~~~~~~~~~~~~~~~~~^
Compiling C++ (in opt mode) /Users/orl/projects/moose/framework/build/unity_src/markers_Unity.C...
1 error generated.
make: *** [/Users/orl/projects/moose/framework/build.mk:144: /Users/orl/projects/moose/framework/build/unity_src/meshgenerators_Unity.arm64-apple-darwin20.0.0.opt.lo] Error 1
make: *** Waiting for unfinished jobs....

I assume that this error is due to the moose-libmesh downgrade from 2023.01.24 build_0 to 2022.11.07 build_2.

Are these downgrades because moose-wasp uses moose-mpich 4.0.2 build_6 instead of build_7 like the others?

If so, could you update moose-wasp to be used with whichever moose-libmesh (and others) the latest devel needs?

Also, how is this usually kept in sync to avoid this kind of situation when dependency package versions are updated?

milljm commented 1 year ago

I'll rebuild the wasp modules after a new rebase against moose devel on my end. We've had a libMesh update since we first started building the moose-wasp module, and I didn't include that when I built this lastest version. I suspect each arch needs to be bumped (which I'll also do).

FYI, this all happens relatively easy, once moose-wasp (if that's what we end up calling it) is added to our "Conda Build ARCH" targets seen here (passing): https://civet.inl.gov/event/109970/.

edit: Correction, a bump to moose-mpich, is what is conflicting here. To be clear, this: moose/conda/wasp/conda_build_config.yaml:

moose_mpich:
  - moose-mpich 4.0.2 build_6   <-- needs to be build_7

loganharbour commented 1 year ago

@brandonlangley we're close here. @milljm and I discussed this morning and we'll try to get moose-wasp built by default and added to moose-test-tools. Therefore, it'll be available by default to most users and we can get this in for most users to use wasp by default.

milljm commented 1 year ago

the latest moose-wasp has been built and made available. There shouldn't be a downgrade when attempting to install it along side an up-to-date moose environment.

brandonlangley commented 1 year ago

@milljm -

the latest moose-wasp has been built and made available. There shouldn't be a downgrade when attempting to install it along side an up-to-date moose environment.

Thank you for updating.

Unfortunately, I'm still getting those same downgrades when I attempt to install moose-wasp on my M1 chip MacBook.

  Package                 Version  Build          Channel                                      Size
─────────────────────────────────────────────────────────────────────────────────────────────────────
  Install:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  + moose-wasp              1.0.0  build_0        conda.software.inl.gov/public/osx-arm64       3MB

  Change:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  - libpq                    15.1  hbce9e56_3     conda-forge                                      
  + libpq                    15.1  hb650857_1     conda-forge/osx-arm64                         2MB
  - moose-libmesh-vtk       9.1.0  h6b51079_17    conda.software.inl.gov/public                    
  + moose-libmesh-vtk       9.1.0  h18806a5_16    conda.software.inl.gov/public/osx-arm64      27MB
  - moose-mpich             4.0.2  build_7        conda.software.inl.gov/public                    
  + moose-mpich             4.0.2  build_6        conda.software.inl.gov/public/osx-arm64       7kB
  - moose-petsc            3.16.6  build_6        conda.software.inl.gov/public                    
  + moose-petsc            3.16.6  build_5        conda.software.inl.gov/public/osx-arm64      25MB

  Downgrade:
─────────────────────────────────────────────────────────────────────────────────────────────────────

  - krb5                   1.20.1  h127bd45_0     conda-forge                                      
  + krb5                   1.19.3  hf9b2bbe_0     conda-forge/osx-arm64                         1MB
  - libcurl                7.87.0  hbe9bab4_0     conda-forge                                      
  + libcurl                7.86.0  hd538317_1     conda-forge/osx-arm64                       332kB
  - libnghttp2             1.51.0  hd184df1_0     conda-forge                                      
  + libnghttp2             1.47.0  h232270b_1     conda-forge/osx-arm64                       835kB
  - libtool                 2.4.7  hb7217d7_0     conda-forge                                      
  + libtool                 2.4.6  hbdafb3b_1008  conda-forge/osx-arm64                       534kB
  - moose-libmesh      2023.01.24  build_0        conda.software.inl.gov/public                    
  + moose-libmesh      2022.11.07  build_2        conda.software.inl.gov/public/osx-arm64      36MB

  Summary:

  Install: 1 packages
  Change: 4 packages
  Downgrade: 5 packages

  Total download: 95MB

─────────────────────────────────────────────────────────────────────────────────────────────────────

I updated my environment and even wiped out my whole mamba install and installed everything fresh just to be sure.

It looks like linux-64 and osx-64 were updated today, but osx-arm64 was last updated Feb. 6 on the public channel.

Does the osx-arm64 version of moose-wasp still need to be built and added?

milljm commented 1 year ago

I don't know how, but I might have uploaded the previous version of Arm. I have deleted it, re-built it, and uploaded it... can you try again?

This time, I checked using an ARM machine to verify that moose-wasp wants moose-mpich build_7...

brandonlangley commented 1 year ago

@milljm -

I have deleted it, re-built it, and uploaded it... can you try again?

I installed the new osx-arm64 version of moose-wasp and built / tested MOOSE and everything works perfect now.

When the moose-wasp environment is active, moose.mk picks up the WASP_DIR and builds using the conda WASP.

So the conda WASP route acts identical to when I run update_and_rebuild_wasp.sh script as a pre-req to building.

This is great - thanks a lot!

I have one disconnect in the tie between your channel update and the conda/wasp/conda_build_config.yaml file.

Even if I don't update the moose-mpich build in my conda/wasp/conda_build_config.yaml source code file from:

moose_mpich:
  - moose-mpich 4.0.2 build_6

to

moose_mpich:
  - moose-mpich 4.0.2 build_7

everything still builds fine using the updated version of moose-wasp you added to the public channel of course.

In other words, it still uses the moose-wasp you made using moose-mpich 4.0.2 build_7 rather than build_6.

What is the relationship between the code in the conda/wasp directory and moose-wasp on the public channel?

Should I update the conda_build_config.yaml moose-mpich build from build_6 to build_7 on this branch?

Sorry this is just due to my ignorance of conda, but what does updating those conda/wasp/* files accomplish?

brandonlangley commented 1 year ago

@loganharbour -

@brandonlangley we're close here. @milljm and I discussed this morning and we'll try to get moose-wasp built by default and added to moose-test-tools. Therefore, it'll be available by default to most users and we can get this in for most users to use wasp by default.

Thanks a lot, this sounds great! I have two quick questions.

How do the CIVET boxes get updated so WASP is used? Do they use moose-test-tools so it'll be there by default?
What's the relationship between moose-test-tools and moose-tools? Because when I setup MOOSE conda, I run:
```
mamba create -n moose moose-tools moose-libmesh
```
using this guide to make a moose environment with moose-tools and moose-libmesh, but not moose-test-tools.

milljm commented 1 year ago

Should I update the conda_build_config.yaml moose-mpich build from build_6 to build_7 on this branch?

@brandonlangley actually, I was just going to ask you if I can make this change (and more that are needed to other moose/conda/ recipes).

What is the relationship between the code in the conda/wasp directory and moose-wasp on the public channel?

The files you mention are only utilized when attempting to build Conda package(s) locally (something Civet does), and Apptainer containers (a completely different topic). In every other way these files are ignored while working in the MOOSE repository, running make, running tests, etc.

The recipes are contained in the MOOSE repository to create a direct correlation of what update_and_rebuild_petsc.sh produces, and what moose-petsc Conda package would be produced.

We have built a script which parses this out for Apptainer purposes (off topic), but I think if you ran it, it might help make some sense:

cd moose/scripts
./versioner.py libmesh --yaml
<trimmed>
conda:
  build: 0
  install: moose-libmesh=2023.01.24=build_0
<trimmed>

According to the above, this current HASH of MOOSE requires this version of the moose-libmesh Conda package. I suspect we will need to add moose-wasp to the list of libraries we keep track of (if we make MOOSE require the wasp submodule).

milljm commented 1 year ago

How do the CIVET boxes get updated so WASP is used? Do they use moose-test-tools so it'll be there by default?

We are going to create an additional step recipe which creates (keeps up to date) the moose-wasp package. Right now, I've been creating it manually so when the time comes to add it, other MOOSE PR's don't fail when they are asked to build moose-wasp.

What's the relationship between moose-test-tools and moose-tools? Because when I setup MOOSE conda, I run:

We plan on placing a dependency for moose-wasp within moose-test-tools. So that when moose-test-tools is installed, so too will moose-wasp. Your question is a good one. I apologize that it requires a lengthy answer!

There is something interesting to note here; Conda tries very hard to install the greater number of latest packages if one does not specifically ask for a package requiring older versions. I think the following installation matrix describes it best (pay close attention to the date versions):

mamba install moose-tools
  + moose-test-tools         2022.12.05  py310h51c69a1_0
  + moose-tools              2023.02.13  py310h51c69a1_0

^Conda avoids installing moose-wasp, by installing an older version of moose-test-tools, which has less constraints for other packages and thus brings in more packages at their latest available version. son-of-a...

mamba install moose-test-tools
  + moose-mpich                   4.0.2  build_7
  + moose-test-tools         2023.02.13  py310h51c69a1_0
  + moose-wasp                    1.0.0  build_0

^Conda adheres to your request by installing the very latest available version of moose-test-tools. And with it, new requirements of also installing moose-wasp, which also requires moose-mpich.

mamba install moose-tools moose-libmesh
  + moose-libmesh            2023.01.24  build_0
  + moose-libmesh-vtk             9.1.0  hfa13f3c_17
  + moose-mpich                   4.0.2  build_7
  + moose-petsc                  3.16.6  build_6
  + moose-test-tools         2023.02.13  py310h51c69a1_0
  + moose-tools              2023.02.13  py310h51c69a1_0
  + moose-wasp                    1.0.0  build_0

^LOL. Conda adheres to your request. You've specifically asked for moose-tools moose-libmesh. With a direct dependency of moose-mpich made by moose-libmesh, Conda can now agree that; installing the latest version of moose-test-tools, which contains moose-wasp, which also contains a dependency to the same moose-mpich version that moose-libmesh is installing... is a good idea.

If this made little sense, you're not alone. It took us years to figure out what and how Conda decides is more important. We have also determined that positional packages matter; mamba install moose-tools python is not the same as mamba install python moose-tools. You are telling Conda, the first package is more important; Solve this first above all else.

brandonlangley commented 1 year ago

@milljm -

Thanks a lot for the explanation.

That mostly all makes sense with the exception of the Apptainer stuff which is okay as I haven't looked into that yet.

I think the main thing that I failed to notice previously is that moose-tools has a dependency on moose-test-tools.

So adding the dependency for moose-wasp to moose-test-tools will implicitly include it with moose-tools as well.

That makes sense now - but unfortunately, I just made a mistake with this branch, misreading your comment above:

actually, I was just going to ask you if I can make this change

accidentally instead as:

actually, I was just going to ask you to make this change

I bumped the moose-mpich from build_6 to build_7 in WASP's conda_build_config.yaml in this commit - https://github.com/idaholab/moose/pull/22018/commits/a0b10c7df4840b319688502ac2c52aeb2f085afa.

But I also rebased on upstream/devel and force-pushed back failing to see you also just added this commit - https://github.com/idaholab/moose/commit/14d879c80005ed0067914038ee98747341ab9a36.

Add moose-wasp as a dependency in test-tools
Because of the requirement of moose-mpich by moose-wasp, it was
necessary to update the HINT section of that build. And due to a
change for moose-mpich, everything must get bumped.

So I accidentally just wiped out your https://github.com/idaholab/moose/commit/14d879c80005ed0067914038ee98747341ab9a36 commit from this branch which was definitely a big mistake on my part.

Do you still have your https://github.com/idaholab/moose/commit/14d879c80005ed0067914038ee98747341ab9a36 change-set locally and could force-push it back here removing my https://github.com/idaholab/moose/pull/22018/commits/a0b10c7df4840b319688502ac2c52aeb2f085afa commit?

Sorry for the hassle.

milljm commented 1 year ago

So I accidentally just wiped out your https://github.com/idaholab/moose/commit/14d879c80005ed0067914038ee98747341ab9a36 commit from this branch which was definitely a big mistake on my part.

Do you still have your https://github.com/idaholab/moose/commit/14d879c80005ed0067914038ee98747341ab9a36 change-set locally and could force-push it back here removing my https://github.com/idaholab/moose/commit/a0b10c7df4840b319688502ac2c52aeb2f085afa commit?

lol, I do, we're fine. I pushed too soon as is anyway. I needed to wait until I added the civet recipes first. Which they are in now. So I will push my changes...

brandonlangley commented 1 year ago

@milljm -

Great - thanks a lot for adding that commit back.

Do you happen to know why the Build PETSc test started failing in the Clang job with your last commit?

I download the Clang job's results as a tarball since the output for that Build PETSc test in CIVET says:

Output too large. You will need to download the results to see this.

and it looks like the configure step failed with this at the bottom of the output for the Build PETSc test:

*******************************************************************************
         UNABLE to CONFIGURE with GIVEN OPTIONS    (see configure.log for details):
-------------------------------------------------------------------------------
Error configuring SUPERLU_DIST with cmake
*******************************************************************************
  File "/data/civet2/build/moose/petsc/config/configure.py", line 467, in petsc_configure
    framework.configure(out = sys.stdout)
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/framework.py", line 1389, in configure
    self.processChildren()
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/framework.py", line 1377, in processChildren
    self.serialEvaluation(self.childGraph)
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/framework.py", line 1352, in serialEvaluation
    child.configure()
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 1191, in configure
    self.executeTest(self.configureLibrary)
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/base.py", line 138, in executeTest
    ret = test(*args,**kargs)
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/packages/SuperLU_DIST.py", line 86, in configureLibrary
    config.package.Package.configureLibrary(self)
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 937, in configureLibrary
    for location, directory, lib, incl in self.generateGuesses():
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 511, in generateGuesses
    d = self.checkDownload()
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 645, in checkDownload
    return self.getInstallDir()
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 407, in getInstallDir
    installDir = self.Install()
  File "/data/civet2/build/moose/petsc/config/BuildSystem/config/package.py", line 1853, in Install
    raise RuntimeError('Error configuring '+self.PACKAGE+' with cmake')
================================================================================
Finishing configure run at Tue, 14 Feb 2023 07:47:10 -0700
================================================================================

ERROR: exiting with code 1

//: Execution in moosebuild completed with return code 1

Here's the Build PETSc test's full output from the tarball results that I downloaded from the Clang job:

01_Build_PETSc.txt

milljm commented 1 year ago

Do you happen to know why the Build PETSc test started failing in the Clang job with your last commit?

Its been passing because its been using pre-existing petsc modules. Now that this PR bumps petsc, its failing to build. I am not sure why as of yet.

milljm commented 1 year ago

@brandonlangley I would ignore this for now. It's an allowed failure.

brandonlangley commented 1 year ago

@milljm -

I would ignore this for now. It's an allowed failure.

Okay thanks - that sounds good.

I see the Create moose-wasp target was added to Conda build (Linux) / Conda build (Intel Mac) / Conda build (ARM Mac).

Just curious, while we have some momentum on this PR, what is the next step to get WASP in use in the CIVET testing?

Right now, tests that try to build MOOSE - like the Build moose test target in the Test install job - all fail with this error:

In file included from /data/civet0/build/moose/framework/contrib/hit/wasp_braceexpr.h:4,
                 from /data/civet0/build/moose/framework/contrib/hit/wasp_braceexpr.cc:2:
/data/civet0/build/moose/framework/contrib/hit/wasp_parse.h:14:10: fatal error: wasphit/HITInterpreter.h: No such file or directory
   14 | #include "wasphit/HITInterpreter.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

With WASP installed, that should go away since I fixed WASP_DIR in moose.mk to match the conda environment variable.

Then we'll be able to see the results of all of the CIVET tests using this WASP-HIT - what needs to happen next for that?

milljm commented 1 year ago

I wasn't going to start creating WASP targets until I see the normal tests passing...

I am no expert, but why is this failing: https://civet.inl.gov/job/1347818/

moose-wasp isn't installed. Shouldn't it be using the old method and pass?

loganharbour commented 1 year ago

I am no expert, but why is this failing: https://civet.inl.gov/job/1347818/

A proper compile using hit when wasp is not available is on my plate to fix. I had a chat with @dschwen about the best way to do this and I'm partially done with it. Just haven't had time to finish. It's on the list for the week.

@brandonlangley needs to look at the conda tests (which have wasp available):

brandonlangley commented 1 year ago

@loganharbour -

@brandonlangley needs to look at the conda tests (which have wasp available):

https://civet.inl.gov/job/1347830/

https://civet.inl.gov/job/1347831/

https://civet.inl.gov/job/1347832/

https://civet.inl.gov/job/1347833/

Thanks for the heads up, I actually noticed those failures earlier in the afternoon and have been looking into them today.

The failures result from two different input errors our WASP lexer is catching - neither are directly from the HIT wrapper.

These two input files:
- moose/modules/porous_flow/test/tests/dirackernels/pls03.i
- moose/modules/porous_flow/test/tests/dirackernels/pls03_action.i
both contain this same parameter defining a function where the value string does not have single-quotes around it:
```
function = if((x<1)&(y>0.5),1E7,-1E7)
```
WASP's lexer currently has separate regular expression rules for single-quoted values vs non-single-quoted values.

The difference between these two is that the characters [, ], #, & are not allowed in non-single-quoted values.

So in the above input files, & is the problem because the if((x<1)&(y>0.5),1E7,-1E7) value is not single-quoted.

This is the first example of an ampersand (or the other characters) being legally allowed in a value that isn't quoted.

All other parameters in inputs that use these characters in a function's value field wrap that string in single-quotes.

Is the above value intended to be allowed without quotes? If so, I can update WASP and look for unintended fallout.
This input file:
- moose/modules/porous_flow/test/tests/flux_limited_TVD_advection/fltvd_2D_blocks.i
contains this Kernel block defining a TimeDerivative subblock which has a period as the first character of its name:
```
[Kernels]
  [.dummy]
    type = TimeDerivative
    variable = dummy
  []
[]
```
WASP's lexer does support both the legacy GetPot [./subblock] notation as well as the HIT [subblock] notation.

But starting a block / subblock name with just . and not ./ isn't currently allowed as this is the first one I've seen.

Is the above [.dummy] subblock name deliberate? Or is this maybe a typo that should just be fixed in that input file?

brandonlangley commented 1 year ago

@loganharbour @milljm -

@brandonlangley needs to look at the conda tests (which have wasp available):

https://civet.inl.gov/job/1347830/

https://civet.inl.gov/job/1347831/

https://civet.inl.gov/job/1347832/

https://civet.inl.gov/job/1347833/

I fixed these issues (described above) in WASP's lexer, and the WASP repo has be updated with the fixes.

So the moose-wasp conda package should pick up these changes and those CIVET jobs should now pass.

It didn't require any changes to this repo, so how can I kick off another CI run manually without a commit?

I tested everything locally using my updated WASP branch, but I'd like to see them pass in CI just to verify.

milljm commented 1 year ago

I am pretty sure a new commit is necessary no? The wasp submodule needs to point to the new reference?

loganharbour commented 1 year ago

I am pretty sure a new commit is necessary no? The wasp submodule needs to point to the new reference?

Ya. The submodule hash needs to be updateed

brandonlangley commented 1 year ago

@loganharbour @milljm -

I am pretty sure a new commit is necessary no? The wasp submodule needs to point to the new reference?

Ya. The submodule hash needs to be updateed

Ahh yes, good point ... doing that now.

moosebuild commented 1 year ago

Job Precheck on b5cf7bc wanted to post the following:

The following file(s) are changed:

conda/libmesh/meta.yaml

The libmesh conda configuration has changed; ping @idaholab/moose-dev-ops

brandonlangley commented 1 year ago

@loganharbour -

FYI - after updating WASP to fix the lexer issues, all the module and unit tests passed for the conda jobs with WASP enabled.

brandonlangley commented 1 year ago

FYI - another small merge conflict popped up again with what was just merged into next.

Once devel picks up the changes, I'll rebase this on devel again to resolve the conflict.

milljm commented 1 year ago

~~There was a PETSc update as I recall. We just need to bump our versions accordingly (for PETSc and everything that depends on PETSc)~~

Never mind, I assumed there was a moose-petsc package update. There was not.

idaholab / moose

Add WASP submodule and set up in HIT input processing implementation #22018