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

Make f20 file with a volume flow input #169

Merged
merged 20 commits into from
Jan 25, 2021

Conversation

Jiangchao3
Copy link
Contributor

test.xlsx

I have written several Matlab scripts for making the f.20 file according to the suggestion from the slack channel. My riverine data is the volume flow (m3/s), and in an excel format (see the test.xlsx). The file Riverflux_distribution is used to calculate the representative edge width and the distribution percentage of each node on the river boundary. The Make_f20_mycode file can be used to calculate the q (m2/s) for each node on the river boundary. In my testing case, I set five riverine flow boundaries (five hydrological stations) (ibtype=22), and each boundary has three nodes. I noticed that there is not a writefort20 file in the msh/private folder, I copied the code from the writefort19 for the writefor20_mycode.

The Make_f20_mycode can work smoothly, but I can not write the f.20 file yet, maybe somewhere is wrong.

I feel sorry that I can not put all my data on the website because of our regulation, if you need these data, I can send them to you privately by email. Actually, it is easy to test these scripts with some open data. You just need use the data cursor to specify the vstart and vend of each river boundary.

I think it may be a good idea that gives an example_12 for guiding to make the f.20 file.

In addition, I think it is necessary to consider what a standard format is used to store the river flow data, excel, or NetCDF?
Once there is a standard, users can convert their own data into this standard form for ease of use!

@krober10nd
Copy link
Collaborator

Thanks! Let’s start the review. First could you place the codes in appropriate places (I.e., get rid of the folder “mycodes”) and put the functions in their respective places following what’s there already?

Second, you mention that the write fort 20 function isn’t working. Could you elaborate more on what isn’t working?

In regard to the volume flux format, I think csv should be fine (I.e x, y, flux)

@Jiangchao3
Copy link
Contributor Author

Hi, @krober10nd, I have put the functions in a more appropriate place. I have added some comments to explain these scripts. I also want to add an example for guiding users to make the f20 file, the Example_12 has not been completed. I will accomplish this example for testing as soon as possible.

@Jiangchao3
Copy link
Contributor Author

This is the data, please check!

image

data.zip

@krober10nd
Copy link
Collaborator

Thanks for the good work here.

  • Could you elaborate more on what the format is for the csv file that the program reads in? This would be useful to also specify in the example file.

For example, for the docstring in Make_f20_volumeflow it could be

The input file format is a column delimited csv file
 % year, month, day, hour, minute, second, volume flow (in m^3/s).
% The  order of the data must be specified in the order in which the 
% riverine boundary nodes appear in the fort.14 file. 
  • Perhaps Make_f20_volumeflow should be named Make_f20_volume_flow.

  • Could you also write a short paragraph to explain the theory behind going from a volume flux to a areal flux (basically describe in words what happens in Riverflux_distribution? This would be extremely useful for the future!

Thanks again for putting this together.

@Jiangchao3
Copy link
Contributor Author

hi @krober10nd , I have added some comments for the functions Make_f20_volume_flow and Riverflux_distribution. Hope the changes make the process more clear.

I can not upload the test_make_f20.csv file to datasets folder, so I put it in the Tests folder.

@krober10nd
Copy link
Collaborator

Thanks! Should 'DT' be a option kwarg argument in Make_f20_volume_flow.m with the default value hourly?

@krober10nd
Copy link
Collaborator

krober10nd commented Jan 14, 2021

It seems that you need to delete the other Make_volumeflux.m file as you changed it's name per my suggestion.

@Jiangchao3
Copy link
Contributor Author

finished.

@krober10nd
Copy link
Collaborator

@zcobell perhaps you could take a look at this? I don't have many models with a river in them nor have I really used this feature much in ADCIRC.

Copy link
Collaborator

@WPringle WPringle left a comment

Choose a reason for hiding this comment

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

Thanks for PR.

I tried running the Example_12 you created and I have couple of requests for it.

  1. So we can run it from Examples directory and have access to csv file in Tests directory, please change preamble to:
addpath('..')
addpath(genpath('../utilities/'));
addpath(genpath('../datasets/'));
addpath(genpath('../m_map/'));
addpath(genpath('../Tests/'));
  1. For the make_bc call to make the river bcs I saw you had some values for vstart, vend etc commented out. Probably you did it through the GUI but it is possible to enter them as arguments into make_bc kwarg. But it seemed to fail, saying that the vstart could not be found on the boundary. Could you update these numbers to work from command line so that example will work correctly out of the box without GUI? This is what I used:
%% hard coding in vstart and vend of each riverine boundary...
%              type cw start end flux River
m = make_bc(m,'outer',1,2206,2107,1,22);
m = make_bc(m,'outer',1,14784,14779,1,22);
m = make_bc(m,'outer',1,763,704,1,22);

Everything else in Example seemed to work fine.

@Jiangchao3
Copy link
Contributor Author

Yes, the vstart and vend are input through the GUI. I have tried using the command line to input the vstart and vend, but it failed as you did. Let me try it again.

@Jiangchao3
Copy link
Contributor Author

@krober10nd and @WPringle ,it is odd that I can successfully use data cursor to identify vstart and vend of each riverine boundary, for example, the vstart is 14784, the vend is 14779. However, when I use the code m = make_bc(m,'outer',1,14784,14779,1,22), it failed. The error is as follows:

image

Somewhere is wrong? Could you help me fix this error?

@krober10nd
Copy link
Collaborator

@Jiangchao3 If you don't use the GUI, then the difference is you need to specify the node number in the mesh. With the data cursor using the GUI, you get the node number on the polygon boundary. This is different from the node number in the global mesh.

You could get the node number in the mesh by doing it once in the GUI/manually, then inspecting the msh.bd.nbvv object. This field contains the node numbers in the global mesh. Specifically, you're interesting in the first and end node of the river boundary segments.

@WPringle
Copy link
Collaborator

@Jiangchao3 If you don't use the GUI, then the difference is you need to specify the node number in the mesh. With the data cursor using the GUI, you get the node number on the polygon boundary. This is different from the node number in the global mesh.

You could get the node number in the mesh by doing it once in the GUI/manually, then inspecting the msh.bd.nbvv object. This field contains the node numbers in the global mesh. Specifically, you're interesting in the first and end node of the river boundary segments.

Yeah, maybe we should change that feature so they are consistent?

@krober10nd
Copy link
Collaborator

Yeah, maybe we should change that feature so they are consistent?

The reason for this is the following: when the GUI is used, only the outer boundary is plotted which is why the click of the data cursor corresponds to the node on the boundary and not the node in the mesh.

@WPringle
Copy link
Collaborator

Yeah, maybe we should change that feature so they are consistent?

The reason for this is the following: when the GUI is used, only the outer boundary is plotted which is why the click of the data cursor corresponds to the node on the boundary and not the node in the mesh.

OK yeah then maybe I try add something in the make_bc help

@Jiangchao3
Copy link
Contributor Author

Got it! Thanks for this information, I have been confused about this error for a while. Thanks for your explanation.

users can enter vstart and vend as arguments into make_bc kwarg
@Jiangchao3
Copy link
Contributor Author

I have add the command lines for identifying vstart and vend. However, I think the cursor GUI method should be recommended rather than the command line method? Because we don't know the node number before using the cursor GUI to identifying the vstart and vend for each riverine boundary.

@krober10nd
Copy link
Collaborator

I have add the command lines for identifying vstart and vend. However, I think the cursor GUI method should be recommended rather than the command line method? Because we don't know the node number before using the cursor GUI to identifying the vstart and vend for each riverine boundary.

Yes, I agree. The mesh may have slight variations in node numbers.

@WPringle
Copy link
Collaborator

WPringle commented Jan 24, 2021

Ok thanks for the update. So regarding the make_bc in the example, the user doesn't actually know where we should specify the river bcs. Just based on your knowledge. So typically what I do is instead of hardcoding the node integers into the make_bc I do a knnsearch to get those integers based on the coordinates we want:

river_points = [-60.0  30.0;
                -60.1  30.1] % example coordinates of river edges
bc_k = ourKNNsearch(m.p',river_points',1);
m = make_bc(m,'outer',1,bc_k(1),bc_k(2),1,22);

That way we can automate it. Because currently even through GUI I do not know where the rivers actually are supposed to be. Can you add these coordinates and lines into the example?

@krober10nd
Copy link
Collaborator

That's a good strategy @WPringle BTW, please note that we'll have to merge the latest main branch before the final merge.

@Jiangchao3
Copy link
Contributor Author

Yes, I agree. this is a good idea to automate the process of making the river boundary. Thanks for your suggestion. I will try to add this function to the Example.

identify the vstart and vend for each riverine boundary automatically using the command-line method
@Jiangchao3
Copy link
Contributor Author

Hi @WPringle and @krober10nd, I think it is great to add the command-line method to identify the vstart and vend automatically. I have added several lines in Example 12 to achieve it. Thanks for your advice again.

@WPringle
Copy link
Collaborator

Example worked out-of-the-box on command line. And the f20 and bd structures of the mesh all look consistent etc. Thanks @Jiangchao3 for your great contribution!

@krober10nd
Copy link
Collaborator

Okay, I've merged the latest Projection branch into @Jiangchao3 fork and updated the changelog, which means everything should be equal (latest HEAD of projection and their fork+branch).

@krober10nd krober10nd merged commit 4adc129 into CHLNDDEV:Projection Jan 25, 2021
@Jiangchao3
Copy link
Contributor Author

Hi @krober10nd and @WPringle, I am very honored to contribute to the OM. You are always helping me and thanks for your guidance. I will continue to use OM in my work. I love this software and the community you created for discussion.

@Jiangchao3 Jiangchao3 deleted the make-f20 branch January 26, 2021 02:10
@krober10nd krober10nd mentioned this pull request Jan 26, 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

Successfully merging this pull request may close these issues.

3 participants