search

EcohydrologyTeam / ClearWater-modules

A collection of water quality and vegetation process simulation modules designed to couple with water transport models.
MIT License
5 stars 1 forks source link

Issue instantiating static variables for nsm1. #66

Closed imscw95 closed 8 months ago

imscw95 commented 9 months ago

I am trying to recreate the tsm model implementation with all the nsm1 variables that we have created. I have been experimenting in the two notebooks in the 'examples' folder titled 'model_architecture.ipynb' (TSM implementation) and 'model_architecture_nsm.ipynb' (NSM implementation). You can see that I compressed all the imports and variable assignments into one code block, respectively, but whereas these values are registered in the tsm_model xarray, the nsm_model xarray comes up empty when I try to recreate this code. I feel like the set up is identical to the tsm set up, just with more static variable dictionaries. The variable objects are also defined in different subfolders, but they are all properly registered to NutrientBudget so not sure why that would be a problem. Made a test_nsm1_static_vars.py file in the main folder to try to recreate the tsm file structure, but that wasn't the issue. One other thing I noticed was that importing the NutrientBudget model was more finicky than the EnergyBudget model. NutrientBudget had to be imported explicitly whereas EnergyBudget was able to be called from cwm.tsm.EnergyBudget. Would appreciate any ideas to troubleshoot.

aufdenkampe commented 9 months ago

@imscw95, thanks for posting this issue. I'll try to take a look, but probably won't have time until Thursday, unfortunately.

xaviernogueira commented 9 months ago

What branch should I look on to see your code? I can check it out.

imscw95 commented 9 months ago

All good @aufdenkampe and thanks @xaviernogueira. Branch is IJM_NSM_nb.

xaviernogueira commented 9 months ago

@imscw95 do you mind running the notebook and committing it (with the cell outputs) it to the branch? I don't have conda on my laptop rn so i want to try first looking at your code/outputs and seeing if I spot a problem. If that doesn't help I'll dig deeper.

imscw95 commented 9 months ago

@xaviernogueira No problem, just did, though I thought I had, and I'm having trouble getting it to render on github. Hope its now showing up for you.

sjordan29 commented 8 months ago

Hi @imscw95, I was holding off at looking at this until merge conflicts with @kewalak's branch were resolved. Where are you at with that? It looks there have been some recent MRs, so I'm happy to start digging into this if the conflicts are resolved.

imscw95 commented 8 months ago

Hey Sarah, sorry we didn't follow up. We did resolve our merge conflicts. However, yesterday I made put all the NSM1 static, dynamic, and processes definitions into a single file, respectively, and successfully initialized the model by recreating the TSM set up. In this way, I am avoiding importing variables/processes from many different folders, which I guess was the issue. This can be seen in the IJM_NSM_nb branch, model_architecture_nsm.ipynb, cell 3. Going through this process, I found a few other corrections of the same variety as the ones Kelsey and I had made in the last merge (duplicate variable definitions, inconsistent variable names in the processes definitions, etc.) so the IJM_NSM_nb branch is most up to date. I am now running into an issue with running a timestep for the nsm model, indicated by the failure of the sorter.py - there is a circular dependence in the DOX processes that I cannot figure out, so if you want to look through that branch and see if you can figure that out, that'd be the best place to work from right now.

sjordan29 commented 8 months ago

Thanks Isaac -- great to hear all the merge conflicts are resolved and you're now able to initialize the model! I can take a look at the sorter later today or tomorrow morning.

sjordan29 commented 8 months ago

Looks like DOX_ApGrowth and DOX_AbGrowth are not getting registered properly as dynamic variables.

If you run print(nsm_model.dynamic_variables_names), those do not appear. I think that's preventing dDOXdt from calculating, and in turn DOX.

I can dig more into this tomorrow, but you might know faster than me the best way to fix that.

sjordan29 commented 8 months ago

Figured it out! There were a few typos in those variables and corresponding processes. I committed my changes to your branch if you want to take a look

Looks like we'll bump into a new issue next -- but I'd recommend closing this issue and opening new ones as we encounter new issues so this one doesn't wander too far from its original intent.

imscw95 commented 8 months ago

Awesome! I see it's now running successfully. Good to close this one.