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

first test case pull request #1

Merged
merged 2 commits into from
Aug 11, 2021
Merged

first test case pull request #1

merged 2 commits into from
Aug 11, 2021

Conversation

sukhreens
Copy link
Collaborator

No description provided.

Copy link
Owner

@cgrudz cgrudz left a comment

Choose a reason for hiding this comment

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

This looks good to me, nice work with the merge and the code cleanup! That's definitely appreciated. I think a nice way to push the test/TestTimeSeriesGeneration.jl file forward is to model this script with the same import statements as in the Euler-Maruyama test. For example, we can use the dx_dt function built-in within the models/L96.jl with an import statement from the DataAssimilationBenchmarks.jl module hierarchy to utilize this optimized definition of the time derivative for the model states. Similarly, we can call the time stepper as in the Euler-Maruyama test case with an import from the methods/DeSolvers.jl module, which can be structured like the existing test case.

I think that calling a test against a data file imported as an artifact will be excessive, as there is a very nice classical way to benchmark the integration methods that solve the time evolution models. This is performed by checking the output against each model simulation against the others to ensure that there is an agreement within tolerated error bounds. This is a nice way that we can extend the development from the test case of the Euler-Maruyama scheme into a test case on all schemes simultaneously. Within short simulations, their differences should grow at expected rates according to the accuracy of the approximation.

The file I/O will also be a good element to test, so that it is verified that the script can successfully write and read back in the experiment output data. This kind of statement is done in Julia with a try / catch statement used for exception handling. We can go over this in an example in the data processing scripts where it checks if it can load a specified file and handles the error if not.

For the most immediate step, you can work on homogenizing the code style with imports from the Euler-Maruyma test case over to the TimeSeriesGeneration module, and I'll have a look for a better structure for the info message so that we don't have to get a splash on every sub-module call.

@cgrudz cgrudz merged commit e6cb17b into cgrudz:master Aug 11, 2021
cgrudz pushed a commit that referenced this pull request Aug 16, 2022
Update c-merchant branch
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