Skip to content

Add rotational viscous damping fix (torque = -gamma * omega)#22

Open
SueHeir wants to merge 2 commits intomainfrom
researcher/rotational-viscous
Open

Add rotational viscous damping fix (torque = -gamma * omega)#22
SueHeir wants to merge 2 commits intomainfrom
researcher/rotational-viscous

Conversation

@SueHeir
Copy link
Copy Markdown
Owner

@SueHeir SueHeir commented Mar 16, 2026

Summary

  • Adds RotationalViscousDef and apply_rotational_viscous system to mddem_fixes, the rotational analog of the existing translational viscous damping
  • Applies damping torque proportional to angular velocity: τ = -γ * ω, removing angular kinetic energy to help particles stop spinning during settling
  • Registered at ScheduleSet::PostForce (same as translational viscous); only active when [[rotational_viscous]] config is present (zero overhead otherwise)
  • Accesses DemAtom omega/torque via AtomDataRegistry

Configuration

[[rotational_viscous]]
group = "all"
gamma = 0.001  # angular damping coefficient

Test plan

  • Damping torque opposes angular velocity direction
  • Zero angular velocity produces zero damping torque
  • Group filtering correctly excludes non-group atoms
  • Torque magnitude scales linearly with gamma and omega
  • All 12 tests pass (cargo test -p mddem_fixes --no-default-features)

🤖 Generated with Claude Code

Implements angular velocity-proportional damping in mddem_fixes, mirroring
the existing translational viscous fix. Registered at PostForce, accesses
DemAtom via AtomDataRegistry for omega and torque fields. Includes 4 tests
covering torque opposition, zero-at-rest, group filtering, and linear scaling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@SueHeir
Copy link
Copy Markdown
Owner Author

SueHeir commented Mar 16, 2026

Maintainer Review: ❌ CHANGES REQUESTED

Good physics implementation, but there's an architectural layering issue.

Issue: mddem_fixesdem_atom dependency

mddem_fixes is an mddem_* (shared infrastructure) crate. This PR adds a dependency on dem_atom (a dem_* domain crate). This violates the project's dependency layering:

  • mddem_* crates should work for both MD and DEM users
  • An MD-only project using mddem_fixes (for addforce, setforce, viscous, gravity) would now pull in dem_atom unnecessarily

Files: crates/mddem_fixes/Cargo.toml (dem_atom dependency added), crates/mddem_fixes/src/lib.rs (DemAtom imported)

Suggested fix

Move RotationalViscousDef and apply_rotational_viscous to a DEM-specific location:

Option A (preferred): Create a small dem_fixes crate for DEM-specific fixes, keeping mddem_fixes clean for shared infrastructure.

Option B: Use a Cargo feature flag in mddem_fixes:

[features]
dem = ["dem_atom"]

Gate the rotational viscous code behind #[cfg(feature = "dem")].

What's good

  • Physics is correct: τ = -γ * ω is standard rotational viscous damping
  • Config pattern ([[rotational_viscous]]) follows conventions
  • Tests are well-written with analytical validation
  • AtomDataRegistry access pattern is correct

The fix itself is clean — it just needs to live in the right place.

Fixes architectural layering: mddem_* (shared infra) should not depend
on dem_* (DEM-specific) crates. Rotational viscous damping needs DemAtom
for omega/torque access, so it belongs in dem_granular.

- Remove dem_atom dependency from mddem_fixes
- Add RotationalViscousPlugin to dem_granular::rotational_viscous
- Re-export from mddem prelude
- All 4 rotational viscous tests preserved and passing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@SueHeir
Copy link
Copy Markdown
Owner Author

SueHeir commented Mar 17, 2026

Fix: Moved rotational viscous from mddem_fixes to dem_granular

Addressed the architectural layering issue:

  • Removed dem_atom dependency from mddem_fixes (stays mddem_* clean)
  • Created dem_granular::rotational_viscous module with its own RotationalViscousPlugin
  • Re-exported from mddem::prelude
  • All 4 tests passing, full workspace builds clean

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.

1 participant