Skip to content

resistive-layer-width sanity checks for SLAYER - #298

Open
amlakbekaluachule wants to merge 4 commits into
feature/tearing-growthratesfrom
FKR-width
Open

resistive-layer-width sanity checks for SLAYER #298
amlakbekaluachule wants to merge 4 commits into
feature/tearing-growthratesfrom
FKR-width

Conversation

@amlakbekaluachule

@amlakbekaluachule amlakbekaluachule commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Added FKR resistive-inertial and Visco-resistive width

@amlakbekaluachule
amlakbekaluachule marked this pull request as draft June 22, 2026 14:58
@amlakbekaluachule
amlakbekaluachule marked this pull request as ready for review June 22, 2026 14:58
@amlakbekaluachule amlakbekaluachule added Cleanup and Clarifications auto-merge Automatically merge PR when CI passes enhancement New feature or request and removed Cleanup and Clarifications auto-merge Automatically merge PR when CI passes labels Jun 22, 2026
@amlakbekaluachule

amlakbekaluachule commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

@logan-nc this PR does two things:

  1. Fixes a bug in slayer_layer_thickness (LayerThickness.jl)
  2. Adds set_resistive_width_based_psihigh (new file,SetResistiveWidthPsihigh.jl)

Let me know if anything looks off.

@d-burg @claude
Tearing - MERGE - Merge feature/tearing-growthrates into FKR-width
8a77155 
Brings FKR-width up to date after 362 commits on feature/tearing-growthrates,
including two develop merges and the InnerLayer relocation.

Two conflicts, both from that relocation. src/Tearing/InnerLayer/SLAYER/ now
lives at src/InnerLayer/SLAYER/ (InnerLayer moved to the top level so it loads
before ForceFreeStates, which calls it for the matched-Delta' Galerkin solve):

  - LayerThickness.jl: the LayerWidths constructor call conflicted because this
    branch added the delta_FKR / delta_visco arguments while the other side
    reindented. Kept both this branch's extra arguments and the target branch's
    formatter indentation. The struct itself merged cleanly and carries all nine
    fields.
  - SetResistiveWidthPsihigh.jl was added inside the renamed directory, so git
    flagged a file-location conflict; accepted its relocation to
    src/InnerLayer/SLAYER/.

Also stripped trailing whitespace from the two touched files, which would fail
the pre-commit hygiene hook.

Verified: the package precompiles clean, LayerWidths carries
(ising, m, n, dels_db, delta_s, delta_s_m, d_beta, delta_FKR, delta_visco), and
runtests_slayer_{params,riccati,inputs,runner}.jl and runtests_innerlayer.jl all
pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@d-burg
d-burg changed the base branch from develop to feature/tearing-growthrates July 31, 2026 00:33
@d-burg

d-burg commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

I synced this branch with feature/tearing-growthrates (merge 8a771555) and retargeted the PR base from develop to feature/tearing-growthrates. The diff now shows your 2 files instead of 65.

Worth knowing why the retarget was needed: LayerThickness.jl doesn't exist on develop at all — the whole SLAYER module is tearing-branch work, and develop only carries a Slayer.jl stub. So the PR couldn't have merged there regardless of how clean it was.

The merge hit two conflicts, both caused by src/Tearing/InnerLayer/SLAYER/ being relocated to src/InnerLayer/SLAYER/ on the parent branch (the InnerLayer module had to move above ForceFreeStates to break a dependency cycle). Nothing to do with your changes — resolved keeping your δ_FKR / δ_visco constructor arguments. The package precompiles and the SLAYER tests pass on the merged branch.

Two things worth a look before review:

1. SetResistiveWidthPsihigh.jl is never loaded

SLAYER.jl includes LayerParameters.jl, Riccati.jl, LayerThickness.jl and LayerInputs.jl (lines 51–54) but not SetResistiveWidthPsihigh.jl, so the helper isn't reachable:

julia> using GeneralizedPerturbedEquilibrium
julia> isdefined(GeneralizedPerturbedEquilibrium.InnerLayer.SLAYER, :set_resistive_width_based_psihigh)
false

Adding include("SetResistiveWidthPsihigh.jl") alongside the others should wire it in, plus an export if it's meant to be public API.

2. δ_FKR / δ_visco aren't in the LayerWidths docstring

The # Fields list in LayerThickness.jl (lines 133–139) still stops at d_beta. The repo convention is to document struct fields in the docstring rather than as inline comments, so these want a line each saying what they are and their units.

Minor: I also stripped some trailing whitespace from both files while resolving the merge — the pre-commit hygiene hook rejects it.

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

Reviewers

@d-burg d-burg Awaiting requested review from d-burg
@jhalpern30 jhalpern30 Awaiting requested review from jhalpern30
@logan-nc logan-nc Awaiting requested review from logan-nc

Assignees

@logan-nc logan-nc

@d-burg d-burg

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amlakbekaluachule @d-burg @logan-nc