-
Notifications
You must be signed in to change notification settings - Fork 19
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
Add diagnostic module #2064
Merged
Merged
Add diagnostic module #2064
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sbozzolo
changed the title
[WORK IN PROGRESS] Add diagnostic module
Add diagnostic module
Sep 6, 2023
Sbozzolo
force-pushed
the
gb/diagnostics_ready
branch
19 times, most recently
from
September 8, 2023 19:03
0e25fb3
to
68b6890
Compare
simonbyrne
reviewed
Sep 8, 2023
Sbozzolo
force-pushed
the
gb/diagnostics_ready
branch
2 times, most recently
from
September 11, 2023 17:32
101a32b
to
275488c
Compare
simonbyrne
reviewed
Sep 11, 2023
Sbozzolo
force-pushed
the
gb/diagnostics_ready
branch
from
September 11, 2023 23:43
069d1b8
to
78a98f8
Compare
We want to use `abbreviations.jl` in submodules too. Therefore, we have to avoid any definition like the one had before this commit (otherwise, we get a redefinition).
Prior to this commit, the error message printed out the computed output/computed iterations as opposed to seconds, which made the error message confusing
Currently, the diagnostic variables are stored in a dictionary. We might want to be able to change the internal representation in the future. For that, we provide an interface to the database of known diagnostic variables through `set_diagnostic_variable!` and `get_diagnostic_variable`. If we want to change the internal representation in the future, we will be able to do so easily.
If we a Vector Field, `eltype` will return `Vector`, not float. So, we have to be more creative with how to get the float_type for general cases.
.= does not work with Vector Fields, but parent() returns the underlying data, so we can directly fill the array with the desired value
The generic constructor for SciMLBase.CallbackSet has to split callbacks into discrete and continuous. This is not hard, but can introduce significant latency. However, all the callbacks in ClimaAtmos are discrete_callbacks, so we directly pass this information to the constructor
The current design leads to exploding compile times. The reason is currently unknown. To avoid these problems, this new design adds one single callback called every time step that calls the diagnostic functions when it is their turn.
Sbozzolo
force-pushed
the
gb/diagnostics_ready
branch
from
September 25, 2023 15:45
9727046
to
b78910f
Compare
bors r+ |
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new diagnostic module that roughly follows what described in #2043.