davidwilby / deepsensor

A Python package for tackling diverse environmental prediction tasks with NPs.
https://tom-andersson.github.io/deepsensor/
MIT License
0 stars 0 forks source link

Addressing Monotonic coordinates error in model.py #6

Closed MartinSJRogers closed 1 week ago

MartinSJRogers commented 1 month ago

When running stitch_clipped_predictions() in model.py, if the patch-wise predictions have overlapping coordinates, an error from xr.combine_by_coords is raised stating that the coordinates of the arrays to be combined are not monotonic in dimension {dim}.

This was being caused by the use of int() in lines 925, 928, 937 and 940. The use of int() was eventually resulting in patches with negative borders, causing the monotonic error to come about.

To address the issue, this PR remove this use of int().

ps, there are a lot of prints() to delete :)

davidwilby commented 1 month ago

ps, there are a lot of prints() to delete :)

Happy for me to do this? If there are any you want to keep they can use logger.debug() instead of print()

MartinSJRogers commented 1 month ago

The removal of the prints should be done now, so I think it should be all good to merge this PR, if you're happy. This is only addressing the Monotonic error, so doesn't relate to the ongoing questions about whether we need to know the coordinate direction of pandas dataframes or not

MartinSJRogers commented 1 month ago

@davidwilby I think I have finally found a way around the issue of not knowing whether the coordinates are ascending or descending if exclusively using pandas. To get round the issue I propose that in data/loader.py we do the following:

  1. Find the minimum and maximum values for x1 and x2 using the pre-existing code: var.index.get_level_values("x1").min() etc
  2. Unnormalise these min and max values using data_processor_unnormalise()
  3. After step 1 and 2, you know the real coordinate values when x1 and x2 are at their minimum and maximum. This means you can work out if x1 and x2 are ascending or descending from the 'top left' in exactly the same way as you do with an xarray. i.e. if the real life coordinate value when x1 is at its minimum is smaller than where x1 is at its maximum, then you know x1 is ascending.

The problem: The data_processor is not currently imported into data/loader.py, so currently these lines of code cause an error. I wasn't sure of the best way to do this, but would you be able to suggest an idea for this last step?

As per your other comments, I have removed all the print()s and I have changed the names to self.coord_ascending_from_top_left and self._compute_coord_ascend_from_top_left() to make it more explicit what is going on and to make the Boolean value make more sense. Happy to chat more! Cheers

davidwilby commented 1 week ago

Closing as this is now fixed by #8 instead.