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

Remove DIST_PREC option #22

Closed
jhamman opened this issue Jul 31, 2013 · 10 comments
Closed

Remove DIST_PREC option #22

jhamman opened this issue Jul 31, 2013 · 10 comments
Assignees
Labels
Milestone

Comments

@jhamman
Copy link
Member

jhamman commented Jul 31, 2013

Description - Remove all loops over dist and dist array dimensions from code.
Optional? - No

@bartnijssen
Copy link
Member

Bart will help out with this as well.

@bartnijssen
Copy link
Member

When removing the DIST_PRCP option we could just get rid of dist_prcp.c and invoke full_energy(), put_data(), and write_model_state() directly from vicNl.c, which would make it easier to read.

@tbohn
Copy link
Contributor

tbohn commented Feb 5, 2014

Yes, I agree. The hard part is in removing all of those loops over the "dist" dimension throughout all the mid-level functions.

@tbohn
Copy link
Contributor

tbohn commented Mar 4, 2014

In making this change, one thing I could potentially do is rename the "dist_prcp" structure to something that doesn't reference the DIST_PREC feature. Ideally, I would think something like "cell" would be most appropriate (and renaming the current "cell" structure to "soil"), since the dist_prcp structure is the top-level container for all states/fluxes of a given cell. However, this would require making tons of changes throughout the code - with opportunities for errors along the way.

Perhaps I should rename it to something else, like "column", or just not rename it at all. Any suggestions?

@bartnijssen
Copy link
Member

Maybe a first step would be a little graphic that shows the dependencies between the different data structures. That will probably quickly point us to the right way of getting rid of DIST_PREC (or renaming it).

I agree that renaming it to the name of an existing structure maybe asking for errors. What does the original CELL structure refer to (this should be easy to see from the diagram).

@tbohn
Copy link
Contributor

tbohn commented Mar 15, 2014

Just an update on my progress. I've removed the dist_prec() function, moving the calls to full_energy(), put_data(), and write_model_state() into main(). I've removed all variables related to the DIST_PRCP option.

I'm in the process of renaming the "dist_prcp" structure to "all_data", since it contains all other data structures (except for atmos) that describe the states/fluxes of the cell control volume.

The final step will be to remove all loops over "dist" and reduce the dimensions of cell_data and veg_var from 3 to 2.

@jhamman
Copy link
Member Author

jhamman commented May 30, 2014

I'm reopening this since I'm seeing a few relic dist_precip references. For example:

  • src/free_dist_prcp.c (in develop and dev_5.0 branches)
  • src/make_dist_prcp.c (in dev_5.0 branch).

These were likely merge issues on my part but they should be cleaned up before we close this issue.

@jhamman jhamman reopened this May 30, 2014
@jhamman
Copy link
Member Author

jhamman commented Sep 25, 2014

I just noticed that initialize_new_storm.c is still there too...

@tbohn
Copy link
Contributor

tbohn commented Sep 25, 2014

Hi guys, are you waiting on me to fix this (it's assigned to me)? Perhaps it would be more appropriate to reassign to Bart, since he did the port from 4.2? Or, am I missing something?

@bartnijssen
Copy link
Member

Not entirely clear to me either. I am pretty sure it's all gone from the version I am working on (though need to check). I'd assign it to Joe for the time being since he alludes to a merge error.

On Sep 24, 2014, at 7:19 PM, Ted Bohn notifications@github.com wrote:

Hi guys, are you waiting on me to fix this (it's assigned to me)? Perhaps it would be more appropriate to reassign to Bart, since he did the port from 4.2? Or, am I missing something?


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants