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

CSD applied to Dansgaard-Oeschger events #48

Merged
merged 13 commits into from
Oct 14, 2023
Merged

CSD applied to Dansgaard-Oeschger events #48

merged 13 commits into from
Oct 14, 2023

Conversation

JanJereczek
Copy link
Contributor

This PR mainly provides an example (docs/src/examples/do-events.jl) of Critical Slowing Down applied on the $\delta^{18}O$ record from NGRIP. This time series displays DO-events and therefore many abrupt transitions in real-world, paleoclimatic data. Furthtermore, the same analysis is performed on simulation data from CLIMBER-X (less noisy and even time sampling), an Earth Model of Intermediate Complexity able to reproduce DO-like events.

Remark: The time steps at which the DO-events are visible in the NGRIP timeseries come from Rasmussen 2014 and should be downloaded in future. For now, hand-coded. In the best case, this should be replaced by an automated procedure like those provided by ChangePoints.jl, which however did not give good results so far on the NGRIP data.

This PR additionally comes with minor suggestions:

  • Use DocumenterCitations.jl as suggested and added refs.bib.
  • The function segmented_significance() was introduced to perform the sliding window analysis such that a single significance value per segment and per indicator is output. This is subject to future changes since I want to introduce a new struct GrowingWindowViewer. We should discuss this live at some point.
  • Replaced docs/src/tutorial.md by docs/src/examples/tutorial.jl such that the tutorial is built by Literate.jl.
  • precompute_ridgematrix was renamed to ridgematrix since it simply computes the matrix.

@JanJereczek
Copy link
Contributor Author

estimate_indicator_changes and significant_transitions now rely on multiple dispatch, by accepting SlidingWindowConfig<:WindowedIndicatorResults (renamed) or the newly introduced SegmentWindowConfig<:WindowedIndicatorResults. The latter requires the user to provide the start and end time of each segment into the config. To avoid any confusion about the functions performing the threaded loops, we now have two (internal) functions for this: sliding_surrogates_loop! and segmented_surrogates_loop!. For the sliding analysis, the t_indicator, t_change are simply vectors. For the segment analysis, they are each a vector of vectors: to me is the most intuitive way to handle the segmentation. Similar for x_indicator, which is a vector of matrices (dimensions of matrix as in in a sliding analysis). x_change is just a matrix with row index = segment index and column index = indicator index. Thus the user only computes one change metric value and therefore p-value per segment.

As suggested, we now use DocumenterCictations and the @cite macro.

Minor corrections were made to the DO-events example, especially the conclusions. It is still somewhat ugly that the transition times are hand-coded and we might want to make that more elegant in the future.

Copy link
Member

@Datseris Datseris left a comment

Choose a reason for hiding this comment

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

Hi Jan, thanks this is great! I am leaving one review, that is small and easy to tackle. I am short on time, so I'll do another review later, but you might as well address this one now if you can.

I agree with all that you said. I also agree that Thus the user only computes one change metric value and therefore p-value per segment. But be sure to state this in the documentation string.

For the example that you have written: start segmenting the code blocks more. Don't have 100-200 l.o.c. in one code block, try to separate them more so that instead of 1 paragraph and then 100 lines of code, there is 1 sentence then 20 loc then another sentence then 20 loc etc.

That's not a fundamental rule or anything. Sometimes its fine to have 100 or 200 locs. But from time to time try to avoid it, especially at the early parts of the tutorial.

src/analysis/surrogates_significance.jl Outdated Show resolved Hide resolved
docs/src/examples/do-events.jl Outdated Show resolved Hide resolved
docs/src/examples/do-events.jl Outdated Show resolved Hide resolved
docs/src/examples/do-events.jl Outdated Show resolved Hide resolved
src/analysis/surrogates_significance-old.jl Outdated Show resolved Hide resolved
src/analysis/windowed_indicators.jl Outdated Show resolved Hide resolved
docs/src/examples/tutorial.jl Outdated Show resolved Hide resolved
@Datseris
Copy link
Member

For the sliding analysis, the t_indicator, t_change are simply vectors. For the segment analysis, they are each a vector of vectors: to me is the most intuitive way to handle the segmentation. Similar for x_indicator, which is a vector of matrices (dimensions of matrix as in in a sliding analysis). x_change is just a matrix with row index = segment index and column index = indicator index. Thus the user only computes one change metric value and therefore p-value per segment

I did not understand that. Can you please try to explain again what you said above? Additionally, please point me to the specific lines of source code where you are talking about so that it is easier for me to follow.

- `t_change`, the time vector of the change metric.

- [`config::SegmentWindowConfig`](@ref), used for the analysis.
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should clarify what I tried to convey when describing how the results of a segment analysis are structured!

)
x_change[k, i] = chametric(z)
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main difference to a sliding window analysis

Copy link
Member

@Datseris Datseris left a comment

Choose a reason for hiding this comment

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

Alright, seems good. There are some crucial documentation improvements we need to do first.

Besides those, I would argue that we should rename SegmentWindowConfig to SegmentedWindowConfig. The window segemenration has already been done by the user.

#=
## Hindcast on NGRIP data

As one can see... there is not much to see so far. Residuals are impossible to simply eye-ball and we therefore use TransitionsInTimeseries.jl to study the evolution, measured by the ridge-regression slope of the residual's variance and lag-1 autocorrelation (AC1) over time. In many examples of the literature, including [boers-early-warning-2018](@cite), the CSD analysis is performed over segments (sometimes only one) of the timeseries, such that a significance value is obtained for each segment. By using `SegmentWindowConfig`, dealing with segments can be easily done in TransitionsInTimeseries.jl and is demonstrated here:
Copy link
Member

Choose a reason for hiding this comment

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

Please reference the type SegmentationWindowConfig with the @ref syntax.

Comment on lines 6 to 7
- [`SlidingWindowConfig`](@ref).
- [`SegmentWindowConfig`](@ref).
Copy link
Member

Choose a reason for hiding this comment

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

I am unhappy about the documentation strings of these two config structs. If you read them, they are almost entirely identical. It's as if they assume the user already knows what these things do. They don't even discuss the differences between the two.

We need to re-write both docstrings. First, in SlidingWindowConfig we need to say that this does an analysis of a sliding window over the input timeseries to estimate the indicators, and then a second sliding window analysis over the indicators to estimate the change metrics (i.e., as in the Tutorial@ref). Then, in SegmentWindowConfing, we shouldn't copy paste stuff. For all arguments that are the same, say "the arguments x, y, z and keywords w, o, p are exaftly like in SlidingWindowConfig@ref.". Then, describe what is the difference of this type. Say that it does sliding window over pre-selected windows by the user, but then computes only a single change metric for this window, and hence, in conrast to SlidingWindowConfig, this type does not have any window parmaeters for the change metric.

t, x, t_indicator, x_indicator, t_change, x_change, config
)
end

"""
estimate_indicator_changes(config::SegmentWindowConfig, x [,t]) → output
Copy link
Member

Choose a reason for hiding this comment

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

This function should NOT have two documentation strings. The function does the same thing. It performs the estimation of indicators and change metrics as instructed by the configuration struct. Hence, we only need one simple docstring that shouldn't be more than 2 or 3 lines of text. The main explanation is happening in the configuration structs. The configuration strucs already say that they are passed into estimate_indicator_cjhanges.

@Datseris Datseris merged commit daa2bbd into main Oct 14, 2023
0 of 2 checks passed
@Datseris
Copy link
Member

@JanJereczek I merged this in. Notice that the PR violated the rule "do only 1 thing", as it actually did three things:

  1. Expanded the API. With this PR we made it possible to add new different types for how to estimate change metrics and indicators.
  2. Added a new way to estimate indicators and change metrics, the segmented window.
  3. Added a new example.

However, that's okay for now, because all thse three things were tightly interlinked and we needed the example to see how the new interface could be used.

@Datseris Datseris deleted the DO-examples branch October 14, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants