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 setting of MOSART_MODE from buildnml #48

Closed
ekluzek opened this issue Nov 2, 2021 · 3 comments · Fixed by #50
Closed

Remove setting of MOSART_MODE from buildnml #48

ekluzek opened this issue Nov 2, 2021 · 3 comments · Fixed by #50

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Nov 2, 2021

In mosart1_0_44 I added an awkward change where MOSART_MODE gets set in buildnml.

Some comments on this are here:

#46 (comment)

We need to decide on a better solution and bring that in when we can.

@ekluzek
Copy link
Contributor Author

ekluzek commented Nov 3, 2021

@billsacks notes there is likely an order dependence in the current operation which is bad. That could be removed by moving the change to buildlib, but it's still weird to have xml variables changed by buildlib.

@billsacks
Copy link
Member

@ekluzek - I never addressed your initial question about having this in buildlib vs. buildnml. Since CLM_ACCELERATED_SPINUP is defined in env_run.xml, it feels wrong to have some dependence on it in buildlib, since then changes after building wouldn't be captured. So I think buildnml is better than buildlib, but both seem problematic.

@ekluzek
Copy link
Contributor Author

ekluzek commented Nov 3, 2021

Ahhh yes. One problem is actually that MOSART_MODE is in env_build, but CLM_ACCELERATED_SPINUP is in env_run. So neither way is technically correct. But, you could argue that since MOSART_MODE is env_build it should be in buildlib. And I suppose we could move CLM_ACCELERATED_SPINUP in env_build as well. But, I still agree that neither way is good here. I do think putting it in env_build removes the potential order dependence issue that you brought up though.

We'll talk more about this tomorrow. I hope we can come up with something that feels better all around.

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

Successfully merging a pull request may close this issue.

2 participants