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

Add stage "assim" and optional obs_seq write to outline #677

Closed
wants to merge 2 commits into from

Conversation

kdraeder
Copy link
Contributor

@kdraeder kdraeder commented May 14, 2024

Description:

The "Detailed program execution" in filter.rst is missing the "assim" stage.
I also added a line about the optional writing of partial obs_seq file contents.

What difference can there be between the "assim" and "output" stages?
It's not the posterior inflation; that's done before "assim".

Fixes issue

#227

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Documentation changes needed?

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.

Tests

I remade the documentation with make html operating on this new branch.

Checklist for merging

  • Updated changelog entry
  • Documentation updated
  • Update conf.py

Checklist for release

  • Merge into main
  • Create release from the main branch with appropriate tag
  • Delete feature-branch

Testing Datasets

  • Dataset needed for testing available upon request
  • Dataset download instructions included
  • No dataset needed

@kdraeder kdraeder added the Documentation Improvements or additions to documentation label May 14, 2024
@kdraeder kdraeder self-assigned this May 14, 2024
Copy link
Member

@hkershaw-brown hkershaw-brown left a comment

Choose a reason for hiding this comment

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

Hi Kevin,

I started reviewing this, the changes are ok. I think the observation writing comment is a bit misleading.

But then I read the whole page

"If you want to try out a different filter type modify the filter code in the assim_tools_mod.f90 file. See the MODULE assim_tools_mod documentation."

what?!

There are broken links "code tree" "DART web pages".

There is a separate high-level da workflow page

There is this 'filters' page that is out-of-date.

I don't think it makes sense at all to have 2 (maybe 3 or more) versions of the same documentation.

Another page:
"controlling which output files are output by filter"

assimilation_code/programs/filter/filter.rst Outdated Show resolved Hide resolved
assimilation_code/programs/filter/filter.rst Outdated Show resolved Hide resolved
assimilation_code/programs/filter/filter.rst Outdated Show resolved Hide resolved
@nancycollins
Copy link
Collaborator

hi kevin, to answer your question about assim vs output files -- the difference is when filter cycles multiple times without exiting, like for the lower order models. the assim files have a time series of states, while output only has the last model state and can be used as a restart file.

@kdraeder
Copy link
Contributor Author

kdraeder commented May 15, 2024

hi kevin, to answer your question about assim vs output files -- the difference is when filter cycles multiple times without exiting, like for the lower order models. the assim files have a time series of states, while output only has the last model state and can be used as a restart file.

Thanks! I had just figured that out and was about to comment on it.
I've tweaked the rst outline to reflect that 'output' is written after the AdvanceTime loop.

@kdraeder
Copy link
Contributor Author

Hi Kevin,

I started reviewing this, the changes are ok. I think the observation writing comment is a bit misleading.

But then I read the whole page
...

It looks like I've opened another can-o-worms . . .
Tell me which of these things (and ...?) should be fixed in this PR versus opening another.

@hkershaw-brown
Copy link
Member

Hi Kevin,
I started reviewing this, the changes are ok. I think the observation writing comment is a bit misleading.
But then I read the whole page
...

It looks like I've opened another can-o-worms . . . Tell me which of these things (and ...?) should be fixed in this PR versus opening another.

I think if you're fixing "Detailed Program Execution of filter" then it needs fixed in the documentation as a whole.

@hkershaw-brown
Copy link
Member

hot take: let's take an hour or so at the CISL in person to sort out the structure of the dart documentation.

@nancycollins
Copy link
Collaborator

something to think about before a documentation discussion: best advice i got from a documentation writer at IBM was decide who your audience is when you're writing each part of the docs. new user, power user, developer, manager/funding agent, etc.

@kdraeder
Copy link
Contributor Author

something to think about before a documentation discussion: best advice i got from a documentation writer at IBM was decide who your audience is when you're writing each part of the docs. new user, power user, developer, manager/funding agent, etc.

Excellent advice!
I also find it helpful to have that information in the documentation that I'm reading,
as a guide to what I should read. The TeX Book does this extensively using some sort of
"hazardous road" sign(s) on sections that go into the weeds.

@kdraeder
Copy link
Contributor Author

kdraeder commented May 15, 2024

Hi Kevin,
I started reviewing this, the changes are ok. I think the observation writing comment is a bit misleading.
But then I read the whole page
...

It looks like I've opened another can-o-worms . . . Tell me which of these things (and ...?) should be fixed in this PR versus opening another.

I think if you're fixing "Detailed Program Execution of filter" then it needs fixed in the documentation as a whole.

I started looking into the "If you want to try out a different filter type"... and quickly found
that I don't know in detail how the filter type is defined in the new QCEFF framework,
so I'm not prepared to fix that section of filter.rst.

Added item about posterior inflation damping.
Moved 'analysis' entry to the correct place (after posterior obs items)
No entries about debugging options.
Clarified item about looping.

Not done: many problems Helen identified.
This will wait until after a strategy meeting (week of 2024-5-20)
@hkershaw-brown
Copy link
Member

Hi Kevin, closing this as we chatted about at the CISL in person week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants