moose
moose copied to clipboard
idaholab
Add external fluid property submodules
refs #27097
CIVET changes to get through:
- [ ] https://github.com/idaholab/civet_recipes/pull/1739
Patches:
- [ ] https://github.inl.gov/ncrc/blue_crab/pull/272
- [ ] https://github.inl.gov/ncrc/dire_wolf/pull/293
- [ ] https://github.inl.gov/ncrc/sockeye/pull/759
- [ ] https://github.inl.gov/ncrc/pronghorn/pull/1266
- [ ] https://github.inl.gov/ncrc/relap-7/pull/2569
- [ ] https://github.inl.gov/idaholab/sabertooth/pull/445
Apps submodules are kept around for the testing recipe to not fail. They are no longer used in compilation though.
TODO:
- [x] finish makefile
- [x] verify local testing is to code OK: extensive unit testing. We could check coverage
- [x] verify documentation
Issues found:
- [x] using the right folder structures for docs (wont show up at the right place)
- only stubs!
- [x] create testing recipes
- [x] Fix non-unity build (missing headers)
- [ ] get testing recipes through
Note:
- if you build the FP moose module, it depends on all contrib FPs if you build a contrib FP, it wont build the other contrib FPs, but still depends on them if there are leftovers from the FP module compilation This could complicate things for testing if moose is built only once
Job Documentation on c143d69 wanted to post the following:
View the site here
This comment will be updated on new commits.
Job Coverage on c143d69 wanted to post the following:
Framework coverage
| 97af19 | #27027 c143d6 | ||||
|---|---|---|---|---|---|
| Total | Total | +/- | New | ||
| Rate | 85.03% | 85.02% | -0.00% | - | |
| Hits | 105193 | 105192 | -1 | 0 | |
| Misses | 18527 | 18528 | +1 | 0 | |
Modules coverage
Fluid properties
| 97af19 | #27027 c143d6 | ||||
|---|---|---|---|---|---|
| Total | Total | +/- | New | ||
| Rate | 85.11% | 85.15% | +0.04% | 85.71% | |
| Hits | 7738 | 7748 | +10 | 6 | |
| Misses | 1354 | 1351 | -3 | 1 | |
Full coverage reports
Reports
-
framework -
chemical_reactions -
combined -
contact -
electromagnetics -
external_petsc_solver -
fluid_properties -
fsi -
functional_expansion_tools -
geochemistry -
heat_transfer -
level_set -
misc -
navier_stokes -
optimization -
peridynamics -
phase_field -
porous_flow -
ray_tracing -
rdg -
reactor -
richards -
scalar_transport -
solid_mechanics -
solid_properties -
stochastic_tools -
thermal_hydraulics -
xfem
Warnings
-
fluid_propertiesnew line coverage rate 85.71% is less than the suggested 90.0%
This comment will be updated on new commits.
Why are we not just putting these inside fluid properties instead of as a submodule?
I m getting close here. Combined, FP, FP submodules build fine and so does doco. Working on the apps again now.
This should be ready EDIT: no optional is failing. So I guess we need a solution / decision for SQA EDIT: might be good now
@cticenhour to sign off on python doco/sqa changes Note that a SQA doco build will require the submodules to be checked out. EDIT: no longer the case, see last commit Only two ways around it if we don't want that:
- change the yaml reader to not error on missing files
- dont include FP submodules in the config.yml sqa section, but then we need to find a way to exclude all the
.mdfiles from any doco build. Remove.yml isn't enough for some reason, I tried
A regular doco build does not require the submodules to be checked out. That is because of the changes in this PR which allow warnings on a number of 'missing' items when a submodule is not checked out
@joshuahansel for fluid properties related things
In considering this in the last few minutes - if only the No Optional recipe is failing, I think we should remove SQA builds from that recipe. SQA is already tested via the Documentation recipe, and then once more at the master branch level. As far as SQA audit requirements, there isn't a scenario where No Optional makes sense for the relevant documentation - all is required to be there. I think we're safe to remove it from No Optional with justification, and you can revert the changes in the last commit.
so that's your preference? Include the FPs in the SQA build and remove the sqa build from no-optional ?
Yes - I prefer that we change the recipe, not the way we do SQA builds. Allowing optional items could lead to incorrect or incomplete SQA documentation. I can push up a PR to remove the arg from the recipe step.
I ll remove the last commit once the civet_recipe change is merged then. thanks!
Ready for review imo let's get this in so I can send an angry email to the editors of wherever I sent the FP module paper for review
@cticenhour it turns out simply having the entry into the config.yml and not having the submodule checked out is enough to fail the doco build. Thoughts? Maybe leave the FPs outside of the SQA doco? https://civet.inl.gov/job/2324795/
We can. I think for Saline we have one test. I'll add that this is a good idea to make sure they keep working (we only test that the executable builds and syntax is well registered from the moose side atm)
Ok, please add some very basic tests that use them. Otherwise, PR looks good to me.
Added the tests. @joshuahansel please let me know if you prefer 1 folder instead for all 6 submodules. The only downside of that is that no examples would show up in the documentation (unless we still copy-paste the input file)
Oh and I don't know if you like to do news entries now or later, but this one needs a news entry.