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

Fix to the calculation of constant outflow rate in the y-direction #16

Merged
merged 27 commits into from
Jan 8, 2020

Conversation

tomgrylls
Copy link
Contributor

The calculation of vout and voutold in modforces.f90 has been corrected.

Plus minor corrections to a typo and error warning messages in modboundary.f90.

@tomgrylls tomgrylls added the bug Something isn't working label Jul 13, 2019
@tomgrylls tomgrylls requested review from bss116 and dmey July 13, 2019 20:07
@tomgrylls tomgrylls self-assigned this Jul 13, 2019
@tomgrylls
Copy link
Contributor Author

Small change to a part of the code that @bss116 and I only added recently for use in a validation case.

@bss116 the reason that I saw this was that I was trying to alter the formulation of the flowrate forcing to make sure that we do not average over the buildings (mask them out). In doing this I noticed that this forcing actually only averages over the outlet plane (can only read this from the code because of the use of ie,ie as opposed to ib,ie where it calls slabsum in subroutine masscorr. Did you have the impression that this forcing was averaging over the entire domain too? As part of this pull request I could change both of these so that it does now do this (it is generally better to have larger spatial averages for these kind of forcings)? But I don't want to change the forcing that you have been using all this time!

The other thing that could also be done as part of this is to change what it averages in the vertical direction. If we always have a block at kb then we may want to make a small edit to the height that is used in masscorr. Or I could make a switch so it can still be compatible with running simulations without these blocks.

@tomgrylls
Copy link
Contributor Author

Also @bss116 I think this explains why the wind direction didn't perfectly match up in that validation simulation - We were averaging along a y-z plane at index ie. I will rerun this with update to confirm.

@bss116
Copy link
Contributor

bss116 commented Jul 17, 2019

@tomgrylls I noticed before that the average velocity of the full domain diverged slightly from what was set by the flowrate, but not by as much to make me concerned about it. In fact I thought it had to do with the blocks not being discounted in the flow rate. However averaging only over the outflow plane could be the reason for it and if it has any impact I would expect more stability in the resulting forcing, and potentially less of the slow transients we experienced. So I definitely support making the change to the forcing.

@bss116
Copy link
Contributor

bss116 commented Jul 17, 2019

The other thing that could also be done as part of this is to change what it averages in the vertical direction. If we always have a block at kb then we may want to make a small edit to the height that is used in masscorr.

This sounds a bit patchy, what if there are blocks at the outlet plane, then this definition would not be right.
I think we should do what you proposed, change the forcing to a volume averaged flow rate, such that the flow rate is (average velocity * fluid volume) (opposed to domain outlet plane in current version), which means we need to calculate fluid volume = (domain volume - block volume) there.
And then also changing the calculated average velocity of the current full field (um, vm) with the blocks masked out. Then these two definitions should match up no matter the blocks.

@tomgrylls
Copy link
Contributor Author

tomgrylls commented Jul 26, 2019

Note - @bss116 and I discussed and a good solution would be to create an additional subroutine in modmpi that calculates volume averaged variables (masking out the buildings). This can then be used for a separate volume average forcing in modforces.

@bss116 bss116 changed the title Fix to the calculation of constant volume flowrate in the y-direction Fix to the calculation of constant outflow rate in the y-direction Nov 27, 2019
@bss116
Copy link
Contributor

bss116 commented Dec 11, 2019

The latest commits address issue #38:

  1. forcing is renamed to outflow forcing (new namoptions: luoutflowr, lvoutflowr in &RUN)
  2. flow rate is now always assigned via namoptions (uflowrate, vflowrate in &PHYSICS), both for coldstart and warmstart
  3. flow rate in namoptions is set as plain velocity, e.g. uflowrate = 2 for an average outflow-plane velocity of u = 2 m/s.

@tomgrylls lastly I wanted to change the use of slabsum to avey_ibm, such that the average is calculated correctly, in case there are buildings at the outflow plane, but I got stuck in trying to reproduce the values from slabsum. what would do you suggest as the equivalent call with avey_ibm? I also noticed that uouttot is used in modboundary.f90 and we can therefore not change these variables directly (and should probably check they are what they are meant to be in modboundary).

@bss116
Copy link
Contributor

bss116 commented Dec 12, 2019

what would do you suggest as the equivalent call with avey_ibm?

@tomgrylls got it. it is call avey_ibm(uout,up(ie,jb:je,kb:ke),ie,ie,jb,je,kb,ke,IIu(ie,jb:je,kb:ke),identity(kb:ke)), where identity is just ones.

@@ -842,7 +844,8 @@ subroutine statsdump
varsxy(:,13) = uwxyik(kb:ke)
varsxy(:,14) = wthlxyk(kb:ke)
varsxy(:,15) = vwxyjk(kb:ke)

varsxy(:,16) = udef
varsxy(:,17) = vdef
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now if we ignore inflow-outflow conditions, are the necessary variables for these flow forcings being read for both cold- and warmstarts (in modstartup.f90)?

@bss116
Copy link
Contributor

bss116 commented Dec 13, 2019

The latest commits change the outflow forcing to exclude the cells occupied by buildings and adds a volume average flow rate forcing, addressing issues #38 and #26.
As part of this, I created new functions calculating the outlet plane area for u and v, and the fluid volume (excluding blocks). Currently this is calculated in every masscorr call, but we really only need it to be calculated in the beginning. Any suggestions in which initialisation routine to include this?
The difference between outflow and volume flow forcing is now just an if statement. Do you think it makes sense to have a separate function for it? We should also do a check that not more than one forcing type is applied. I will create a new issue for this.

@bss116
Copy link
Contributor

bss116 commented Dec 20, 2019

A quick update: @tomgrylls and I talked about the flow rate calculations. I will make a few changes and run a couple more tests and then this should be good to go into 0.1.0. Will let you know as soon as this is ready.

@bss116 bss116 added this to the 0.1.0 milestone Dec 22, 2019
@bss116
Copy link
Contributor

bss116 commented Jan 3, 2020

I have now completed the tests for fixed outflow rate and new volume flow rate. I used a symmetric test geometry with four blocks and tested separately a forced flow along u and v wind direction for both outflow rate and volume flow rate. There is some variability between the cases but the pressure gradient forcing, average wind profiles, vertical fluxes etc. look reasonably similar that I think the forcing is doing what it is supposed to do.

@tomgrylls Do you think this is good to go? If so I will go ahead and merge it.
testblocks
uout
uvol
vout
vvol

@bss116
Copy link
Contributor

bss116 commented Jan 3, 2020

Sorry the labels are white, if you click on the image you can see which test case it is.

@bss116
Copy link
Contributor

bss116 commented Jan 4, 2020

@dmey Could it be that there is something wrong with travis for mac again? I ran these simulations on mac and it worked just fine.

@dmey
Copy link
Contributor

dmey commented Jan 4, 2020

@dmey Could it be that there is something wrong with travis for mac again? I ran these simulations on mac and it worked just fine.

@bss116 should be now fixed in 9620185.

@tomgrylls
Copy link
Contributor Author

@bss116 I have not had time to go through the source code in detail but I think that the discussed and displayed tests should provide a very good indication of all that is in this pull request from my understanding. I am happy with these results so will approve.

@tomgrylls
Copy link
Contributor Author

Ah I cannot review as is my pull request

@dmey
Copy link
Contributor

dmey commented Jan 7, 2020

@bss116 if you are OK with this can you please approve and squash merge?

@bss116 bss116 self-requested a review January 7, 2020 17:17
@bss116 bss116 merged commit 7022e59 into master Jan 8, 2020
@bss116 bss116 deleted the tomgrylls/vflowrate branch January 8, 2020 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants