arts icon indicating copy to clipboard operation
arts copied to clipboard

Inconsistent documentation

Open riclarsson opened this issue 2 years ago • 1 comments

Describe the bug vmr_field is defined to have the shape [species, p_grid, lat_grid, lon_grid]. (We do not have species as a variable, so if this is meant to be abs_species, then that should be specified. This is not the current issue, it has the size of abs_species which should perhaps be documented as the fix is done for this issue)

lat_grid is set to empty when the atmosphere is 1D but otherwise must have a size.

lon_grid is set to empty when the atmosphere is 1D or 2D but otherwise must have a size.

This inconsistency is clearly nonsense.

vmr_field should never have the shape [species, p_grid, 0, 0]. It should have at minimum the shape [species, p_grid, 1, 1] to contain any data. So the documentation of vmr_field is wrong.

When the AtmFieldsCalc method takes a zero-sized vmr_field_raw, it outputs a vmr_field of the shape [0, 0, 0, 0], even though there are pressure points. By the documentation, the shape should be [0, p_grid, 0, 0]. By practical reasons the shape should be [0, p_grid, 1, 1]. This seems easiest to achieve by having both internal and external FieldFromGriddedField take the atmosphere_dim variable and fix the output accordingly.

Anyways, this inconsistency leads to a direct bug in DisortCalcIrradiance. It assumes that the shape is at least [species, p_grid, 1, 1], which would be consistent with standard assumption that there may be some data. However, this assumption is clearly wrong since a 0-shape for latitude and longitude means we can have no data in 1D and 2D atmospheres.

To Reproduce Steps to reproduce the behavior:

  1. Checkout PR #554 by @j-petersen
  2. Run the test having compiled in a mode that activates ARTS_ASSERT
  3. See the error in matpack as the MatrixView cannot be created from the vmr_field

Expected solution @m-brath and @erikssonpatrick you two need to discuss through how you wish to define the atmospheric grids here. Clearly the disort integration is wrong by current documentation. Clearly current documentation is also wrong as we need a minimum of some data for 1D and 2D calculations too. This comes together in also fixing the various ways that vmr_field can be generated so that it consistently checks the atmosphere_dim instead of just the gridded field array to determine the final size.

So two tasks:

  1. Ensure that documentation is consistent across the board wrt to the shape of output parameters. I am sure that the false documentation of vmr_field may also be present in other fields because of copy-paste errors.
  2. Decide how you solve this for Disort. We shouldn't need temporaries to work around these kinds of problems as long as the input is OK. At the very least, Disort should check all its input fields that they are OK for what it assumes to be a 1D atmosphere.

riclarsson avatar Oct 28 '22 07:10 riclarsson