Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DHC Network Compatibility with Plug Flow Pipe #2449

Closed
khinkelman opened this issue Apr 13, 2021 · 8 comments
Closed

DHC Network Compatibility with Plug Flow Pipe #2449

khinkelman opened this issue Apr 13, 2021 · 8 comments

Comments

@khinkelman
Copy link
Contributor

Hi @AntoineGautier,

Several base models for the distribution networks (within Buildings.Experimental.DHC.Networks.BaseClasses) have a replaceable pipe model of Fluid.Interfaces.PartialTwoPortInterface.

When trying to implement Buildings.Fluid.FixedResistances.PlugFlowPipe, I received errors relating to the connection with the vectorized ports_b[]. Here is one example:

As pictured below, in Buildings.Experimental.DHC.Networks.BaseClasses.PartialConnection1Pipe, pipDis has a connect statement connect(pipDis.port_b,junConSup.port_1).
When replacing the replaceable pipDis with a plug flow pipe, the connect statement in the base class is no longer valid and needs to be instead connect(pipDis.ports_b[1],junConSup.port_1).

I wonder if there is a way to revise the base classes to accept pipes with both single and vectorized ports? Alternatively, I wonder why the plug flow pipe has a vectorized downstream port and ask if it would make sense to revise the pipe model to have a single outlet port?

Since you've been developing the distribution network models, I leave this issue to you; but please feel free to ask me for help if there is anything you would like me to do. Thanks!

image

@AntoineGautier
Copy link
Contributor

@khinkelman Thanks. I agree that the vectorized port at the pipe outlet is peculiar and I cannot see which use case it simplifies really, or enables to handle in a numerically efficient way as the documentation states. I would also vote for refactoring Fluid.FixedResistances.PlugFlowPipe with a single outlet port. @mwetter What is your thought on that?

@mwetter
Copy link
Member

mwetter commented Apr 13, 2021

The model FixedResistances.PlugFlowPipe has a control volume at the outlet port. This volume takes into account the thermal capacity of the pipe wall. It has a vectorized port so that branches can directly connect to the control volume inside, which allows avoiding the T-piece in the above figure and through this leads to fewer and simpler systems of equations.

I would argue that the pipe material heat capacity is generally small compared to the water heat capacity, in which case the volume could be eliminated, and then a single port could be used, allowing the replaceable model structure above. This could be achieved by using directly FixedResistances.BaseClasses.PlugFlowCore. We could consider renaming this to FixedResistances.PlugFlow. If we do this, we should probably suggest to rename the base classes in IBPSA in particular BaseClasses.PlugFlow which then has the same name but different functionality.

@bravache
Copy link
Contributor

bravache commented May 26, 2021

@mwetter I would be glad to do that renaming/moving since I am working with the PlugFlowPipe currently, but after that refactor, we would get two similarly named models PlugFlowPipe and PlugFlow, which virtually represent the same object, except that PlugFlowPipe has the additional pipe capacity embedded and a vectorized outlet port. Would that be acceptable in terms of redundancy?

If we move forward with that plan, I'd suggest LosslessPlugFlow as a new name for the BaseClasses.PlugFlow model.

@mwetter
Copy link
Member

mwetter commented May 27, 2021

@bravache : I think FixedResistances.PlugFlowPipe and FixedResistances.PlugFlow (which is your new model if I understand right) is not clear enough to the user. Is there a clearer name you can think of?

BaseClasses.LosslessPlugFlow seems fine for BaseClasses.PlugFlow.

@bravache
Copy link
Contributor

bravache commented Jun 1, 2021

@mwetter : The new model I created is called MultiSegmentBuriedPipe.

In my message above, I was following up on your suggestion to rename FixedResistances.BaseClasses.PlugFlowCore to FixedResistances.PlugFlow, which would result in both PlugFlow and PlugFlowPipe models to exist under FixedResistances package. I was wondering if that was your suggestion, or if you had something else in mind.

@mwetter
Copy link
Member

mwetter commented Jun 2, 2021

@bravache : Let's discuss on our coordination call. A possible outline may be

FixedResistances
  - LosslessPipe
  - Pipe
  - PlugFlowPipe
  - ...
  -BaseClasses
   - Pipe
   - PlugFlow
   - PlugFlowCore
   - PlugFlowHeatLoss
   - PlugFlowTransportDelay
   [Maybe best would be to have 
      - FixedResistences
        - Pipe
        - LosslessPipe
        - ...
        - PlugFlow
          - Pipe
          - PipeDiscretized
          - BaseClasses
            - ... (all plug flow base classes)

- BuriedPipes
  - MultiPlugFlowPipe [this would be better if
       PlugFlowPipeDiscretized to use the same
       terminology as for DryCoilDiscretized, DoorDiscretized etc.,
        e.g., use Discretized suffix rather than Multi prefix]
  - GroundCoupling
    [ Should MultiPlugFlowPipe and GroundCoupling be combined into one model called 
    FixedResistances.PlugFlow.BuriedPipe? Would this be most convenient for the user?]

   Or another structure that is more oriented on the component rather than the physics is
      - FixedResistences
        - PressureDrop
        - Junction
        - ...
        - PipesDucts
          - BuriedPipe
          - HydraulicDiameter
          - Lossless
          - PlugFlow
          - PlugFlowDiscretized
          - 

(Please also save as script that uses https://simulationresearch.lbl.gov/modelica/buildingspy/development.html#buildingspy.development.refactor.move_class as we will need to do the same for the IBPSA library.)

@bravache
Copy link
Contributor

bravache commented Jun 2, 2021

Thanks Michael. Following our discussion, I have submitted an issue on the IBPSA repo, and we can discuss this at the next IBPSA meeting.

I am on board with your proposed structure.

@mwetter
Copy link
Member

mwetter commented Oct 25, 2021

This is now on the master of the Buildings library, and a PR is in progress for the IBPSA library, ibpsa/modelica-ibpsa#1537

@mwetter mwetter closed this as completed Oct 25, 2021
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

No branches or pull requests

4 participants