Skip to content

Conversation

@dschoenach
Copy link

@dschoenach dschoenach commented Jan 9, 2025

Useful to have, especially for satellite radiances.

Was requested by Reima in Metcoop.

@dschoenach dschoenach changed the title Add plots 'Obsfit (Active Obervations Only)' Add plot 'Obsfit (Active Obervations Only)' Jan 9, 2025
@ewhelan ewhelan self-assigned this Jan 9, 2025
@ewhelan ewhelan added the enhancement New feature or request label Jan 9, 2025
@ewhelan
Copy link
Contributor

ewhelan commented Jan 9, 2025

Looks good to me.

  1. I presume all well tested.
  2. I might request some documentation ...

@ewhelan ewhelan self-requested a review January 9, 2025 12:40
Copy link
Contributor

@ewhelan ewhelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dschoenach

Thanks for this contribution. I assume all is well-tested and works at your end. Could you look at adding a description of this plot to the documentation. It might inspire others to contribute!

@dschoenach
Copy link
Author

Hi @dschoenach

Thanks for this contribution. I assume all is well-tested and works at your end. Could you look at adding a description of this plot to the documentation. It might inspire others to contribute!

Actually not so well tested I am afraid:
It takes quite a bit of time for more cycles. Possibly due to R's slow data.frame format.
The new functions group the raw data by DTG, varname and level and compute for each group the rms and mean of fg and an.

Have to check if that can be speed up somehow.

@dschoenach dschoenach force-pushed the feature/obsfit_active_obs branch 2 times, most recently from 5082dd5 to 4df0e4d Compare January 9, 2025 17:12
Faster computations by converting tibble to data.table
Load library data.table in init.R
@dschoenach dschoenach force-pushed the feature/obsfit_active_obs branch from 4df0e4d to 4a427ae Compare January 9, 2025 17:22
@dschoenach
Copy link
Author

Computation speed looks now good

But I think it needs loading library data.table in init.R

Could you make test @ewhelan

@dschoenach
Copy link
Author

It should work by placing data.table in init.R, with the standard install script handling the installation.

Ready to merge from my side.

@ewhelan
Copy link
Contributor

ewhelan commented Feb 7, 2025

Will try to test this soon. Struggling with port forwarding on my container. Will hopefully get some help next week.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants