Skip to content

Fix load_operator for LinearNonlinearParamOperator#40

Open
Ady0333 wants to merge 1 commit intogridap:mainfrom
Ady0333:fix/load-operator-typo
Open

Fix load_operator for LinearNonlinearParamOperator#40
Ady0333 wants to merge 1 commit intogridap:mainfrom
Ady0333:fix/load-operator-typo

Conversation

@Ady0333
Copy link
Copy Markdown
Contributor

@Ady0333 Ady0333 commented Apr 2, 2026

Fixes #39

Problem

The load_operator function for LinearNonlinearParamOperator in src/RB/RBSteady/PostProcess.jl (line 184) calls a non-existent function _fixed_operator_parts instead of the correct _load_fixed_operator_parts.

Current broken code:

function load_operator(dir, feop::LinearNonlinearParamOperator; label="")
  feop_lin = get_LHS_operator(feop)
  trial, test = _fixed_operator_parts(dir, feop_lin; label)  # ❌ function doesn't exist
  # ...
end

This is a copy-paste typo where the _load_ prefix was dropped. The correct function _load_fixed_operator_parts is defined at line 146 in the same file.

Evidence this is wrong:

  • The transient version (src/RB/RBTransient/PostProcess.jl:55) correctly calls RBSteady._load_fixed_operator_parts
  • The generic steady version (line 168) also uses _load_fixed_operator_parts
  • Only the LinearNonlinearParamOperator specialization has the typo

Impact

Who is affected: Anyone working with steady nonlinear ROM problems (Navier-Stokes, nonlinear elasticity, etc.) who saves a reduced operator and tries to reload it.

What breaks:

  • Every call to load_operator on a LinearNonlinearParamOperator crashes with UndefVarError: _fixed_operator_parts not defined
  • The save/load workflow for steady nonlinear ROMs is completely broken
  • The examples framework has a try/catch that silently swallows this error and recomputes the entire offline phase instead of loading, wasting potentially hours of computation every session

Silent failure scenario: Users think their saved data doesn't exist or is corrupted, when the load function itself is just broken. The save path works fine only loading fails.

How to Reproduce

using GridapROMs

# Run any steady nonlinear ROM workflow (e.g., Navier-Stokes)
rbop = reduced_operator(rbsolver, feop, fesnaps)

# Save the operator - this works
save(dir, rbop)

# Try to load it back - this crashes
load_operator(dir, feop)  # where feop::LinearNonlinearParamOperator

# ERROR: UndefVarError: `_fixed_operator_parts` not defined

In the examples framework, this manifests as silent recomputation instead of loading precomputed data.

Fix

Changed: src/RB/RBSteady/PostProcess.jl, line 184

Added the missing _load_ prefix to the function name:

function load_operator(dir, feop::LinearNonlinearParamOperator; label="")
  feop_lin = get_LHS_operator(feop)
  trial, test = _load_fixed_operator_parts(dir, feop_lin; label)  # ✅ correct function name
  # ...
end

One line, one prefix. The save path already worked correctly only the load path was broken.

Testing

Added test/RBSteady/save_operator.jl with regression tests that verify:

  1. _load_fixed_operator_parts exists in RBSteady module
  2. _fixed_operator_parts (the typo) does NOT exist
  3. load_operator has a method for LinearNonlinearParamOperator
  4. ✅ The source code of that method calls the correct function name

The fourth check is particularly important it prevents someone from "fixing" the crash by defining the wrong function instead of fixing the actual typo.

Test strategy: Uses compile-time verification instead of full integration testing (which would require heavy FEM setup). This keeps tests fast while ensuring the bug can't be reintroduced.

After This Fix

  • ✅ Steady nonlinear ROM operators can be saved and loaded correctly
  • ✅ The examples framework will correctly reuse precomputed offline data
  • ✅ Users can deploy ROMs using the standard offline-save-load-online pattern
  • ✅ No more silent wasted computation from recomputing instead of loading

_fixed_operator_parts → _load_fixed_operator_parts (typo caused
UndefVarError on every call to load_operator for nonlinear problems).

Signed-off-by: Ady0333 <adityashinde1525@gmail.com>
@Ady0333
Copy link
Copy Markdown
Contributor Author

Ady0333 commented Apr 2, 2026

Hello @nichomueller ,
Please review this pr and let me know if any change is needed.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 45.99%. Comparing base (c1efb5c) to head (4d5fe74).

Files with missing lines Patch % Lines
src/RB/RBSteady/PostProcess.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #40   +/-   ##
=======================================
  Coverage   45.99%   45.99%           
=======================================
  Files         112      112           
  Lines       15063    15063           
=======================================
  Hits         6928     6928           
  Misses       8135     8135           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: NormalSampling uses Uniform distribution instead of Normal in generate_param

1 participant