moltres icon indicating copy to clipboard operation
moltres copied to clipboard

Remove deprecated MOOSE code

Open cticenhour opened this issue 2 years ago • 4 comments

CIVET has flagged Moltres as having deprecated code: https://civet.inl.gov/job/1027414/

Notably, the following error is seen on most of the failing tests:

Deprecated code:
'InversePowerMethod' executioner is deprecated in favor of 'Eigenvalue' executioner.
Few parameters such as 'bx_norm', 'k0', 'xdiff', 'max_power_iterations', 'min_power_iterations', 'eig_check_tol', 'sol_check_tol', and 'output_before_normalization' are no longer supported.
However, 'Eigenvalue' executioner supports more solving options by interfacing SLEPc.

And the following is seen in the laminar_flow.boussinesqSquare test:

Deprecated code:
The parameter p is deprecated.
The coupled variable parameter 'p' has been deprecated and will be removed 1/1/2022. Please use the 'pressure' coupled variable parameter instead.

This issue can be closed when:

  • [ ] The deprecated code outlined in the test results linked above have been addressed and the "Test deprecated" target completes successfully.

Moltres members with sufficient permissions should be able to turn on the "Test deprecated" target manually on the PR testing screen to ensure this condition is satisfied. Feel free to reach out if you have any questions!

cticenhour avatar Apr 26 '22 15:04 cticenhour

Thanks for the heads up! The p parameter is easy enough to replace. The transition to Eigenvalue executioner will require more manhours because we'd have to change our Action files.

On your last point, would we have to manually turn on "Test deprecated" in every PR?

smpark7 avatar May 02 '22 18:05 smpark7

On the CIVET user interface, if you have sufficient permissions, you can turn on tests that don't normally run on PRs. For example I am also a maintainer of Zapdos, so if I look at an open PR, I can also see the option to turn on the deprecated test that would only otherwise run on the master branch: image Do you see this when looking at an open PR results page on CIVET for Moltres? I could check and then submit "Test deprecated" as an add-on for that Zapdos PR above.

cticenhour avatar May 02 '22 19:05 cticenhour

I would only add this for the PR you submit to close this.....you don't necessarily want it running all the time, just when you're trying to clean up what is currently being flagged.

cticenhour avatar May 02 '22 19:05 cticenhour

Oh I initially misunderstood you but I get what you mean now. Thanks for clarifying!

smpark7 avatar May 02 '22 19:05 smpark7

@cticenhour Is InversePowerMethod no longer deprecated? I noticed that the CIVET deprecation tests are passing, and there are some open issues about the new Eigenvalue executioner (e.g., https://github.com/idaholab/moose/issues/20095)

smpark7 avatar Feb 28 '23 19:02 smpark7

You are correct. There seems to be some issues regarding the new executioner, so InversePowerMethod is no longer deprecated for the moment. If you have fixed up everything else, I would say this issue can be closed.

cticenhour avatar Feb 28 '23 21:02 cticenhour

Thanks for the update. We'll track the eventual transition to Eigenvalue with #177.

smpark7 avatar Feb 28 '23 21:02 smpark7