Issue3618 air filter #3986

SenHuang19 · 2024-09-02T21:25:52Z

This PR addresses the comments.

SenHuang19 · 2024-09-04T13:01:17Z

@JayHuLBL Could you please review this PR?

JayHuLBL · 2024-11-06T19:18:02Z

Buildings/Fluid/AirFilters/BaseClasses/Characteristics/filtrationEfficiencyParameters.mo

@@ -0,0 +1,27 @@
+within Buildings.Fluid.AirFilters.BaseClasses.Characteristics;
+record filtrationEfficiencyParameters


Change name to FiltrationEfficiencyParameters

The record name has been changed.

JayHuLBL · 2024-11-06T19:19:45Z

Buildings/Fluid/AirFilters/BaseClasses/Characteristics/filtrationEfficiencyParameters.mo

@@ -0,0 +1,27 @@
+within Buildings.Fluid.AirFilters.BaseClasses.Characteristics;
+record filtrationEfficiencyParameters
+  "Record for filtration efficiency vs. relative mass of the contaminant captured by the filter"


use versus, rather than vs.

Replace versus with vs.

JayHuLBL · 2024-11-06T19:20:46Z

Buildings/Fluid/AirFilters/BaseClasses/Data/Generic.mo

+    "Name of trace substance";
+  parameter
+    Buildings.Fluid.AirFilters.BaseClasses.Characteristics.filtrationEfficiencyParameters
+    filterationEfficiencyParameters


may use shorter parameter name, like filEffPar

Replace filterationEfficiencyParameters with filEffPar

JayHuLBL · 2024-11-06T19:21:32Z

Buildings/Fluid/AirFilters/BaseClasses/Data/Generic.mo

+<p>
+The record contains one dataset: 
+relative mass of the contaminant
+(see <a href=\"modelica://Buildings.Fluid.AirFilters.BaseClasses.FiltrationEfficiency\">


is it the right link to the right class? I think it should like to Buildings.Fluid.AirFilters.BaseClasses.Characteristics.filtrationEfficiencyParameters?

Or you may mean The record contains one dataset <Characteristics.filtrationEfficiencyParameters> for <Buildings.Fluid.AirFilters.BaseClasses.FiltrationEfficiency>?

I meant the definition of the relative mass is provided in modelica://Buildings.Fluid.AirFilters.BaseClasses.FiltrationEfficiency.
I update the document to avoid confusions.

JayHuLBL · 2024-11-06T20:41:05Z

Buildings/Fluid/AirFilters/BaseClasses/Characteristics/filtrationEfficiencyParameters.mo

+  annotation (Documentation(info="<html>
+<p>
+Data record that describes the relative mass of the contaminant captured by the filter <code>rat</code> versus
+the filtration efficiency <code>eps</code>.


Data record that describes the relative mass of the contaminant <code>rat</code> captured by the filter when it has the filtration efficiency specified as <code>eps</code>.

Update it as suggested

JayHuLBL · 2024-11-06T20:54:19Z

Buildings/Fluid/AirFilters/BaseClasses/Data/Generic.mo

+  extends Modelica.Icons.Record;
+  parameter Real mCon_nominal(
+    final unit = "kg")
+    "Maximum mass of the contaminant can be captured by the filter";


Maximum mass of the contaminant that can be ...

Fix the typo.

JayHuLBL · 2024-11-06T21:00:43Z

Buildings/Fluid/AirFilters/BaseClasses/FiltrationEfficiency.mo

    "Relative mass of the contaminant captured by the filter"
    annotation (Placement(transformation(extent={{100,40},{140,80}})));
-


add a protected section to find out the total types of the contaminant

protected parameter Integer nConSub = size(per.substanceName,1) "Total types of contaminant substances";

and then
change above y[size(per.substanceName,1) to y[nConSub], and below the for loop.

Change it as suggested

JayHuLBL · 2024-11-06T22:48:14Z

Buildings/Fluid/AirFilters/BaseClasses/MassAccumulation.mo

-    "Maximum mass of the contaminant captured by the filter";
+  parameter Integer nin(
+    min=1)=1
+    "Number of input connections";


I think you mean the total types of contaminant substances? So can we change the nin to nConSub and change the comment to Total number of contaminant substance types?

Change it as suggested

JayHuLBL · 2024-11-06T22:49:52Z

Buildings/Fluid/AirFilters/BaseClasses/MassAccumulation.mo

+  parameter Integer nin(
+    min=1)=1
+    "Number of input connections";
+  parameter Buildings.Fluid.AirFilters.BaseClasses.Data.Generic per


I think there is no need to have the per parameter. We just need the nominal contaminant mass. Why don't we keep the mCon_nominal?

Change it as suggested

JayHuLBL · 2024-11-06T22:51:56Z

Buildings/Fluid/AirFilters/BaseClasses/FlowCoefficientCorrection.mo

@@ -1,26 +1,28 @@
 within Buildings.Fluid.AirFilters.BaseClasses;
 model FlowCoefficientCorrection
  "Component that calculates the flow coefficient correction factor"
-  parameter Real b
-    "Resistance coefficient";
+  parameter Buildings.Fluid.AirFilters.BaseClasses.Data.Generic per


Do we need to instantiate the per parameter in the base class? I think it may be enough to have the per been instantiated only in the top level class, i.e. Buildings.Fluid.AirFilters.Empirical.

Change it as suggested

JayHuLBL · 2024-11-06T22:55:48Z

Buildings/Fluid/AirFilters/BaseClasses/MassTransfer.mo

+    annotation (Placement(transformation(extent={{100,40},{140,80}})));
+
+protected
+  parameter Real s1[:,:]= {


we can add the parameter nConSub in the protected section and defined it as nConSub=size(substanceName,1). This will avoid calculating the size(substanceName,1) multiple times.

Change it as suggested

JayHuLBL · 2024-11-06T23:01:32Z

Buildings/Fluid/AirFilters/Empirical.mo

 </li>
 <li>
-The <code>extraPropertiesNames</code> has to be defined in the medium model.
+when the <code>extraPropertiesNames</code> in the medium model don't contain all the contaminants
+defined in the <code>per</code>.
 </li>
 </ul>
 </html>", revisions="<html>


Do we reference for the model development?

We don't have a reference.

JayHuLBL

@SenHuang19 Please see the inline comments.

SenHuang19 · 2024-11-07T14:35:16Z

@SenHuang19 Please see the inline comments.

Thanks a lot! I am working on addressing them now.

JayHuLBL · 2024-11-12T23:27:56Z

@SenHuang19 Is it ready for review?

SenHuang19 · 2024-11-13T03:37:41Z

@SenHuang19 Is it ready for review?

@JayHuLBL Yes it is.

SenHuang19 added 8 commits August 25, 2024 23:32

refactoring and adding an example

2a98e8a

script update

5d10a8e

example update

6c49dab

ref dataset update

63142ce

model doc update and small issues fix

db1b8b0

model doc improve

021d6b7

typos fix

a11b196

minor fix

7e1f587

minor doc update

07ad4e9

JayHuLBL reviewed Nov 6, 2024

View reviewed changes

JayHuLBL requested changes Nov 6, 2024

View reviewed changes

Huang added 3 commits November 8, 2024 10:53

model doc enhance and move the per to the top level class

f649536

model doc improve

d3a9dfe

eliminate unnecessary calculations

5fd768e

Huang added 2 commits November 12, 2024 22:02

minor model doc update

2f7c2f2

update the file name

b731d7a

fix typo

d53922b

JayHuLBL merged commit 929e92b into lbl-srg:issue3618_airFilter Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue3618 air filter #3986

Issue3618 air filter #3986

SenHuang19 commented Sep 2, 2024

SenHuang19 commented Sep 4, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024 •

edited

Loading

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024 •

edited

Loading

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL Nov 6, 2024

SenHuang19 Nov 13, 2024

JayHuLBL left a comment

SenHuang19 commented Nov 7, 2024

JayHuLBL commented Nov 12, 2024

SenHuang19 commented Nov 13, 2024

		@@ -0,0 +1,27 @@
		within Buildings.Fluid.AirFilters.BaseClasses.Characteristics;
		record filtrationEfficiencyParameters

		"Relative mass of the contaminant captured by the filter"
		annotation (Placement(transformation(extent={{100,40},{140,80}})));

Issue3618 air filter #3986

Issue3618 air filter #3986

Conversation

SenHuang19 commented Sep 2, 2024

SenHuang19 commented Sep 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayHuLBL Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayHuLBL Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JayHuLBL left a comment

Choose a reason for hiding this comment

SenHuang19 commented Nov 7, 2024

JayHuLBL commented Nov 12, 2024

SenHuang19 commented Nov 13, 2024

JayHuLBL Nov 6, 2024 •

edited

Loading

JayHuLBL Nov 6, 2024 •

edited

Loading