Closed
Conversation
# Separator and Internals - Improvements Summary
This document outlines the improvements made to separator and internals handling in NeqSim, along with remaining suggestions for future work.
---
## Implemented Changes
### ✅ 1. Replaced System.out.println with Proper Logging
**Files updated:**
- `GasScrubberSimple.java` - Uses inherited logger from `Separator`
- `GasScrubberMechanicalDesign.java` - Added logger field and imports
- `SeparatorMechanicalDesign.java` - Added logger field and imports
All debug output now uses `logger.debug()` instead of `System.out.println()`.
### ✅ 2. Added Input Validation to Setters
**DemistingInternal.java:**
- `setArea(double)` - throws `IllegalArgumentException` if area ≤ 0
- `setEuNumber(double)` - throws `IllegalArgumentException` if euNumber < 0
**PrimarySeparation.java:**
- `setInletNozzleDiameter(double)` - throws if diameter ≤ 0
**InletVane.java:**
- `setGeometricalExpansionRatio(double)` - throws if ratio ≤ 0
**InletVaneWithMeshpad.java:**
- `setVaneToMeshpadDistance(double)` - throws if distance < 0
- `setFreeDistanceAboveMeshpad(double)` - throws if distance < 0
**InletCyclones.java:**
- `setNumberOfCyclones(int)` - throws if count < 1
- `setCycloneDiameter(double)` - throws if diameter ≤ 0
**DemistingInternalWithDrainage.java:**
- `setDrainageEfficiency(double)` - throws if not in range [0, 1] (already existed)
### ✅ 3. Fixed Bug in calcTotalLiquidCarryOver
**Problem:** `SeparatorMechanicalDesign.calcTotalLiquidCarryOver()` called a non-existent 4-parameter `calcLiquidCarryOver()` method.
**Solution:** Changed to use the new `calcEfficiency()` method with proper cascaded efficiency calculation:
```java
for (DemistingInternal internal : demistingInternals) {
totalCarryOver *= (1.0 - internal.calcEfficiency());
}
```
### ✅ 4. Added calcEfficiency() Method to DemistingInternal
New method that calculates separation efficiency as `1 - exp(-k * area)`:
```java
public double calcEfficiency() {
double calibrationConstant = 0.5;
return 1.0 - Math.exp(-calibrationConstant * area);
}
```
### ✅ 5. Fixed DemistingInternalWithDrainage Override
**Problem:** Had a deprecated 4-parameter `calcLiquidCarryOver()` with `@Override` annotation pointing to non-existent parent method.
**Solution:** Replaced with proper `calcEfficiency()` override that improves efficiency based on drainage:
```java
@OverRide
public double calcEfficiency() {
double baseEfficiency = super.calcEfficiency();
double improvedEfficiency = baseEfficiency + (1.0 - baseEfficiency) * drainageEfficiency;
return Math.min(improvedEfficiency, 1.0);
}
```
---
## Remaining Suggestions for Future Work
### 1. Carry-Over Correlation Validation
### Current State
The carry-over calculations in `DemistingInternal`, `InletVane`, and `InletVaneWithMeshpad` use simplified correlations with hardcoded calibration constants.
```java
// DemistingInternal.java
double calibrationConstant = 0.5; // Can be adjusted based on experimental data
double carryOverFactor = Math.exp(-calibrationConstant * area);
```
### Suggested Improvements
- **Add configurable calibration constants** as constructor parameters or setters
- **Implement industry-standard correlations** (e.g., Stokes, Souders-Brown)
- **Add unit test fixtures** with experimental validation data
- **Document correlation sources** and applicable operating ranges
```java
// Proposed enhancement
public class DemistingInternal {
private double calibrationConstant = 0.5;
public void setCalibrationConstant(double k) {
this.calibrationConstant = k;
}
public void useCorrelation(CarryOverCorrelation correlation) {
// Apply industry-standard correlation
}
}
```
---
## ~~2. Droplet Size Distribution~~ (Future Work)
### Current State
Carry-over calculations assume uniform droplet behavior. No droplet size distribution is modeled.
### Suggested Improvements
- Add `DropletSizeDistribution` class to model real inlet conditions
- Implement cut-size calculations for cyclones and vanes
- Support Rosin-Rammler or log-normal distributions
- Calculate grade efficiency curves for internals
```java
// Proposed new class
public class DropletSizeDistribution {
private double medianDiameter; // d50
private double spreadParameter; // for Rosin-Rammler
public double getFractionBelowSize(double diameter) { ... }
public double calcCutSize(SeparationDevice device) { ... }
}
```
---
## ~~3. Pressure Drop Coefficient Units Clarification~~ (Future Work)
### Current State
The `DemistingInternal` uses Euler number for pressure drop, but the relationship between Euler number and pressure drop coefficient is not consistently documented.
```java
// Current: Δp = Eu × ρ × v²
// Some references use: Δp = K × (1/2) × ρ × v²
```
### Suggested Improvements
- Add clear Javadoc explaining the coefficient convention used
- Provide conversion methods between Euler number and pressure drop coefficient (K-factor)
- Consider adding a `PressureDropModel` enum to specify which convention is used
---
## ~~4. Serialization for Primary Separation~~ (Future Work)
### Current State
`PrimarySeparation` and subclasses mark the `separator` field as `transient`:
```java
protected transient Separator separator;
```
This means the separator reference is lost during serialization/deserialization.
### Suggested Improvements
- Implement custom `readObject`/`writeObject` methods to restore references
- Or use a post-deserialization hook pattern like `ProcessSystem` uses
- Document the serialization behavior clearly
---
## ~~5. Missing Validation in Setters~~ ✅ IMPLEMENTED
### ~~Current State~~ (Before Fix)
Some setters lacked input validation:
```java
// InletVane - no validation on expansion ratio
public void setGeometricalExpansionRatio(double geometricalExpansionRatio) {
this.geometricalExpansionRatio = geometricalExpansionRatio;
}
```
### Resolution
All setters now validate inputs and throw `IllegalArgumentException` for invalid values.
See "Implemented Changes" section above for details.
---
## ~~6. Integration with Separator Run Method~~ (Future Work)
### Current State
Primary separation and demisting internals are configured but not automatically used in the `Separator.run()` calculation. The carry-over calculations must be invoked manually.
### Suggested Improvements
- Add option to automatically apply primary separation carry-over during `run()`
- Integrate demisting pressure drop into overall separator pressure drop
- Add `setAutoCalculateCarryOver(boolean)` flag
```java
// In Separator.run()
if (autoCalculateCarryOver && primarySeparation != null) {
double carryOver = primarySeparation.calcLiquidCarryOver();
setLiquidCarryoverFraction(carryOver);
// Apply to outlet streams...
}
```
---
## ~~7. Builder Pattern for Complex Configurations~~ (Future Work)
### Current State
Configuring a separator with multiple sections and internals requires many method calls:
```java
scrubber.addSeparatorSection("inlet vane", "vane");
scrubber.getSeparatorSection("inlet vane").setCalcEfficiency(true);
scrubber.getSeparatorSection("inlet vane").getMechanicalDesign().setNominalSize("DN 500");
// ... many more calls
```
### Suggested Improvements
Implement a builder pattern for cleaner configuration:
```java
GasScrubber scrubber = GasScrubber.builder("HP Scrubber")
.withInletStream(inlet)
.withDiameter(2.0)
.withLength(6.0)
.withPrimarySeparation(new InletVaneWithMeshpad("vane", 0.1, 0.3, 0.25))
.addSection("inlet vane", SectionType.VANE)
.withEfficiency(0.95)
.withNominalSize("DN 500")
.addSection("demister", SectionType.MESHPAD)
.build();
```
---
## ~~8. Factory Methods for Common Configurations~~ (Future Work)
### Current State
Users must manually configure all separator components.
### Suggested Improvements
Add factory methods for common industry configurations:
```java
public class SeparatorFactory {
public static GasScrubber createStandardVerticalScrubber(
String name, StreamInterface inlet, double pressure) {
// Pre-configured with typical vane + meshpad
}
public static ThreePhaseSeparator createAPI12JSeparator(
String name, StreamInterface inlet) {
// Configured per API 12J standard
}
public static GasScrubber createHighPressureScrubber(
String name, StreamInterface inlet) {
// Optimized for HP service
}
}
```
---
## ~~9. Performance Metrics and Reporting~~ (Future Work)
### Current State
Limited visibility into separation performance. Must manually query individual components.
### Suggested Improvements
Add a comprehensive performance report:
```java
public class SeparatorPerformanceReport {
private double overallCarryOver;
private double totalPressureDrop;
private Map<String, Double> sectionEfficiencies;
private double gasLoadFactor;
private double kValue;
private boolean isWithinDesignLimits;
public String generateReport() { ... }
public void exportToJson(Path path) { ... }
}
// Usage
SeparatorPerformanceReport report = scrubber.getPerformanceReport();
System.out.println(report.generateReport());
```
---
## ~~10. Unit Consistency~~ (Future Work)
### Current State
Some methods use inconsistent units or undocumented unit conventions:
```java
// Volume sometimes in cm³, sometimes in m³
double volumetricFlow = getInletGasVolumetricFlow(); // m³/s (after /1e5 conversion)
```
### Suggested Improvements
- Add unit specification to all physical property methods
- Consider using a units library or unit-aware return types
- Add overloaded methods with explicit unit parameters
```java
public double getInletGasVolumetricFlow() { return getInletGasVolumetricFlow("m3/s"); }
public double getInletGasVolumetricFlow(String unit) { ... }
```
---
## ~~11. Logging and Diagnostics~~ ✅ IMPLEMENTED
### ~~Current State~~ (Before Fix)
Limited logging for debugging separation calculations. Some `System.out.println` statements remained:
```java
// In GasScrubberSimple
System.out.println("Ktot " + (1.0 - ktotal));
```
### Resolution
All `System.out.println` statements in production code replaced with `logger.debug()` calls.
See "Implemented Changes" section above for details.
---
## ~~12. Thread Safety for Cached Properties~~ (Future Work)
### Current State
Inlet properties are cached during `run()` and accessed by separation devices:
```java
private double inletGasVelocity = 0.0;
private double gasDensity = 0.0;
// etc.
```
### Suggested Improvements
For multi-threaded simulations, consider:
- Making cached properties volatile
- Using atomic references
- Or documenting thread-safety requirements
---
## ~~13. Enhanced Test Coverage~~ (Future Work)
### Current State
`GasScrubberTest` provides good basic coverage but lacks:
- Edge case testing (zero flow, single phase)
- Performance regression tests
- Boundary condition tests
### Suggested Improvements
Add test cases for:
```java
@test
void testZeroLiquidContent() { ... }
@test
void testHighVelocityCarryOver() { ... }
@test
void testMultipleCyclonesScaling() { ... }
@test
void testSerializationRoundTrip() { ... }
@test
void testCarryOverConvergence() { ... }
```
---
## ~~14. Documentation Improvements~~ (Future Work)
### Current State
- Javadoc exists but some methods lack parameter descriptions
- No usage examples in class-level documentation
- Physical assumptions not always documented
### Suggested Improvements
- Add `@example` or code snippets in class-level Javadoc
- Document physical assumptions and limitations
- Add references to equations/correlations used
- Create a "theory" section in wiki documentation
---
## ~~15. Configuration Persistence~~ (Future Work)
### Current State
No built-in way to save/load separator configurations.
### Suggested Improvements
Add JSON/YAML configuration support:
```java
// Save configuration
scrubber.saveConfiguration("scrubber_config.json");
// Load configuration
GasScrubber loaded = GasScrubber.loadConfiguration("scrubber_config.json", inletStream);
```
---
## Priority Matrix (Updated)
| Improvement | Impact | Effort | Status |
|-------------|--------|--------|--------|
| ~~Logging improvements~~ | Medium | Low | ✅ **DONE** |
| ~~Input validation~~ | Medium | Low | ✅ **DONE** |
| ~~Bug fix: calcTotalLiquidCarryOver~~ | High | Low | ✅ **DONE** |
| Carry-over correlation validation | High | Medium | Future |
| Integration with run() method | High | Low | Future |
| Builder pattern | Medium | Medium | Future |
| Factory methods | Medium | Medium | Future |
| Performance reporting | Medium | Medium | Future |
| Droplet size distribution | High | High | Future |
| Unit consistency | Medium | Medium | Future |
| Configuration persistence | Low | Medium | Future |
| Thread safety | Low | Medium | Future |
---
## Summary
### Completed (December 2025)
- ✅ Replaced `System.out.println` with proper logging (3 files)
- ✅ Added input validation to 8 setter methods (6 files)
- ✅ Fixed bug in `calcTotalLiquidCarryOver()` method
- ✅ Added new `calcEfficiency()` method to `DemistingInternal`
- ✅ Fixed broken `@Override` in `DemistingInternalWithDrainage`
### Remaining Work
- Carry-over correlation validation with experimental data
- Automatic integration with separator `run()` method
- Builder pattern and factory methods
- Droplet size distribution modeling
- Enhanced test coverage
---
*Document created: December 2025*
*Updated: December 2025 - Implementation complete*
*Based on code review of NeqSim separator and internals packages*
…into suggestions-even
…into scrubber-updates
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request: Separator Internals and Demister Improvements
Summary
This PR enhances the separator and gas scrubber equipment in NeqSim with improved mechanical design capabilities, better internals modeling, and comprehensive documentation.
Changes
New Classes - Separator Internals
DemistingInternalDemistingInternalWithDrainagePrimarySeparationInletVaneInletVaneWithMeshpadInletCyclonesEnhanced Separator Capabilities
Separator.java- Added methods for demisting internal configuration and reportingSeparatorMechanicalDesign.java- Extended with demisting internals list and total demisting area calculationsGasScrubberMechanicalDesign.java- Improved loggingCode Quality Improvements
System.out.printlnwith properlogger.debug()logging throughoutIllegalArgumentExceptionfor invalid valuesDocumentation
docs/wiki/separators_and_internals.md- Comprehensive guide on separator internalsdocs/SEPARATOR_INTERNALS_IMPROVEMENTS.md- Summary of improvements and future suggestionsTests
GasScrubberTest.java- New test class for gas scrubber functionalitytest_scrubbers.ipynb- Jupyter notebook examples for scrubber simulationsTechnical Details
The new internals framework allows users to:
Example Usage
Files Changed
Separator.javaSeparatorMechanicalDesign.javaDemistingInternal.javaDemistingInternalWithDrainage.javaPrimarySeparation.javaInletVane.javaInletVaneWithMeshpad.javaInletCyclones.javaGasScrubberTest.javaseparators_and_internals.mdTesting
Related Issues
This PR addresses improvements to separator modeling capabilities in NeqSim.
Checklist