Skip to content

Conversation

@wilfonba
Copy link
Collaborator

@wilfonba wilfonba commented Nov 9, 2025

User description

Description

This PR adds an example case with automated scripts to create convergence test results for a 1D advection case. It also fixes a toolchain error that resulted in reduced convergence when pi was used for analaytic patches.

Type of change

  • Something else

Scope

  • This PR comprises a set of related changes with a common goal

How Has This Been Tested?

oldConvergence.pdf


PR Type

Enhancement, Bug fix


Description

  • Add 1D convergence test example with automated scripts

    • Includes case configuration generator, plotting utilities, and job submission script
  • Fix toolchain error with pi constant in analytic patches

    • Remove hardcoded pi substitution to prevent convergence reduction
  • Provide documentation and setup instructions for convergence testing


Diagram Walkthrough

flowchart LR
  A["1D Convergence Example"] --> B["case.py: Config Generator"]
  A --> C["submitJobs.sh: Job Automation"]
  A --> D["plot.py: Error Analysis"]
  A --> E["README.md: Documentation"]
  F["Toolchain Fix"] --> G["Remove pi Substitution"]
  G --> H["Preserve Analytic Accuracy"]
Loading

File Walkthrough

Relevant files
Enhancement
case.py
1D convergence test case configuration generator                 

examples/1D_convergence/case.py

  • New Python script to generate JSON case configuration for 1D two-fluid
    convergence simulation
  • Supports command-line arguments for WENO order, model equations,
    Riemann solver, and grid resolution
  • Configures numerical domain, time stepping, and initial conditions
    with sinusoidal density/volume fraction profiles
  • Sets up two-fluid system with periodic boundary conditions
+81/-0   
plot.py
Convergence error analysis and visualization tool               

examples/1D_convergence/plot.py

  • New Python script to analyze convergence test results across multiple
    grid resolutions and WENO orders
  • Computes L1, L2, and L∞ error norms by comparing simulation results to
    initial conditions
  • Generates log-log convergence plots with reference slopes for orders
    1, 3, and 5
  • Exports error data to CSV file for further analysis
+70/-0   
submitJobs.sh
Automated job submission script for convergence tests       

examples/1D_convergence/submitJobs.sh

  • New bash script to automate execution of convergence test simulations
  • Iterates over multiple grid resolutions (32 to 1024 points) and WENO
    orders (1, 3, 5)
  • Creates directory structure and submits jobs to MFC toolchain with
    appropriate parameters
  • Configurable for different model equations and Riemann solvers
+27/-0   
Documentation
README.md
Convergence test example documentation and setup guide     

examples/1D_convergence/README.md

  • New documentation file explaining the convergence test example setup
    and usage
  • Provides instructions for enabling execution permissions and
    configuring paths
  • Documents customizable parameters and output generation with plotting
    script
  • Explains how to run simulations and generate convergence analysis
    plots
+11/-0   
Bug fix
case.py
Fix pi constant handling in analytic patches                         

toolchain/mfc/case.py

  • Remove hardcoded pi constant substitution in analytic patch
    expressions
  • Prevents incorrect pi value replacement that was causing convergence
    reduction
  • Allows pi to be properly evaluated in mathematical expressions within
    case definitions
+1/-1     

Copilot AI review requested due to automatic review settings November 9, 2025 19:04
@wilfonba wilfonba requested review from a team and sbryngelson as code owners November 9, 2025 19:04
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 9, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

L2 norm is computed as mean of absolute differences (via sqrt of squared diffs), which equals L1; should use sqrt of mean squared error or discrete 2-norm. Also Inf norm sums maxima of two fields, potentially overstating error; consider max over combined fields instead of sum.

## 2 norm
errors[i, j, 0] = 1 / N[i] * np.sum(np.sqrt((sim_a1.y - exact_a1.y) ** 2))
errors[i, j, 0] += 1 / N[i] * np.sum(np.sqrt((sim_a2.y - exact_a2.y) ** 2))

## 1 norm
errors[i, j, 1] = 1 / N[i] * np.sum(np.abs(sim_a1.y - exact_a1.y))
errors[i, j, 1] += 1 / N[i] * np.sum(np.abs(sim_a2.y - exact_a2.y))

## Inf norm
errors[i, j, 2] = np.nanmax(np.abs(sim_a1.y - exact_a1.y))
errors[i, j, 2] += np.nanmax(np.abs(sim_a2.y - exact_a2.y))
Regression Risk

Removal of 'pi' replacement may break existing analytic patches using 'pi'. Confirm that 'pi' is intentionally removed and that examples use explicit '2pix' strings which will now not be replaced unless 'pi' is defined elsewhere.

    'e' : f'{math.e}',
}.get(match.group(), match.group())
Portability

Script contains hardcoded absolute paths for ROOT_DIR and MFC_DIR, making it non-portable. Consider using env vars or relative paths and checking for required executables.

#ROOT_DIR=<WORKING DIRECTORY>
#MFC_DIR=<MFC ROOT DIR>
ROOT_DIR="/Users/benwilfong/Documents/software/MFC-Wilfong/examples/1D_convergence"
MFC_DIR="/Users/benwilfong/Documents/software/MFC-Wilfong"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes support for the pi constant in analytical patch expressions and adds a new 1D convergence example that demonstrates spatial accuracy testing. The removal of pi from the rhs_replace dictionary in the case parser is problematic because the new example and existing examples use pi in analytical expressions.

  • Removes pi constant support from the analytical patch expression parser
  • Adds a 1D two-component advection convergence test example with automation scripts
  • Provides plotting utilities for analyzing convergence rates

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
toolchain/mfc/case.py Removes pi from the rhs_replace dictionary that maps variable names in analytical expressions
examples/1D_convergence/submitJobs.sh Adds shell script to automate running convergence tests across multiple grid resolutions and WENO orders
examples/1D_convergence/plot.py Adds Python script to compute and visualize L1, L2, and Linf error norms
examples/1D_convergence/case.py Adds case file for 1D two-fluid advection test with analytical initial conditions using sinusoidal patterns
examples/1D_convergence/README.md Adds documentation explaining how to run the convergence test

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.02%. Comparing base (bfd732c) to head (103ef16).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1030   +/-   ##
=======================================
  Coverage   46.02%   46.02%           
=======================================
  Files          67       67           
  Lines       13437    13437           
  Branches     1550     1550           
=======================================
  Hits         6185     6185           
  Misses       6362     6362           
  Partials      890      890           

☔ 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

Development

Successfully merging this pull request may close these issues.

1 participant