search

atmtools / konrad

Implementation of a radiative-convective equilibrium model.
MIT License
19 stars 18 forks source link

RCE class modifies input atmosphere #224

Open olemke opened 1 month ago

olemke commented 1 month ago

The RCE class changes the values of the input atmosphere: https://github.com/atmtools/konrad/blob/21e6c81f08f0ba1b093d5e6167c291296031f56b/konrad/core.py#L409

This can have unexpected side effects if a user wants to do several RCE runs (even with newly created RCE instances) consecutively with the same input atmosphere.

To prevent side effects, it would be preferable to make a deepcopy of the input atmosphere when initializing the RCE class: https://github.com/atmtools/konrad/blob/21e6c81f08f0ba1b093d5e6167c291296031f56b/konrad/core.py#L157

If this is not desirable for other reasons, the documentation of the atmosphere parameter should at least contain a clear warning that this class will modify the input variable.

lkluft commented 1 month ago

So far, the reasoning has simply been that it seems obvious that the model run will change the atmospheric state. However, I see that one might expect the initial state and the internal model state to be independent.

I think it is fine to just do a deep copy at initialization (this needs to be done for the surface as well). It might actually break scripts of users who have adapted to the current behavior, i.e. expecting the atmosphere to change. But the output files aren't affected, and the change from atmosphere to rce.atmosphere seems doable.