Skip to content

Forecasting code cleanup#285

Merged
JeffreyCovington merged 27 commits into
2.0-betafrom
Forecasting-Code-Cleanup
Jun 30, 2026
Merged

Forecasting code cleanup#285
JeffreyCovington merged 27 commits into
2.0-betafrom
Forecasting-Code-Cleanup

Conversation

@JeffreyCovington

Copy link
Copy Markdown
Contributor

Miscellaneous cleanup which includes:

  • Renaming likelihoods to be more explicit.
  • Adding more validation checks.
  • Adding better type annotations.
  • Adding and updating documentation.
  • Fixing typos.

Includes ForecastSimulator, ParticleFilterSimulator, and EnsembleKalmanFilterSimulator as top-level interfaces.
…naries of unknown parameters to have consistent usage of keys when iterating over them.
…hood with an `EnsembleKalmanFilterSimulator`.
… likelihood in terms of standard deviation for consistency. Added and updated documentation and annotations.
…naries of unknown parameters to have consistent usage of keys when iterating over them.
…hood with an `EnsembleKalmanFilterSimulator`.
… likelihood in terms of standard deviation for consistency. Added and updated documentation and annotations.

@JavadocMD JavadocMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking much better, thanks! Just a few minor things, see inline.

Comment thread epymorph/forecasting/likelihood.py Outdated
Comment thread epymorph/forecasting/likelihood.py
Comment thread epymorph/forecasting/pipeline.py
Comment thread epymorph/forecasting/likelihood.py
@Averydx

Averydx commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Is it possible to split up some of the code in pipeline.py? I'm having difficulties with the dreaded circular imports. For example some of the dataclasses I'm using for type hinting like UnknownParam and PipelineOutput.

@JavadocMD

Copy link
Copy Markdown
Contributor

Is it possible to split up some of the code in pipeline.py?

There are a number of tricks to accomplish this. For instance, notice the epymorph.tools.data module defines its own Output as a Protocol rather than importing the real Output from epymorph.simulator.basic.output. Breaking out of circular imports was the main reason for doing that.

You can also move certain foundational entities to a third module, if they don't require many imports themselves.

There are certain guidelines that can help organize imports; I thought about enforcing them with tools at one point. Anyway, I'm happy to chat about specifics if it's helpful.

@Averydx

Averydx commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Thank you! That’s very helpful.

Averydx and others added 2 commits May 20, 2026 09:10
* Plotting functionality.

* Error correction.

* Typing

* modified:   epymorph/forecasting/filter_plot.py

* Finally?
* Refactor filter to reduce code repetition.

* Added support for non-Gaussian likelihoods in the EnKF.

* Added support for observing multiple strata.

* Allow unknown params to use wildcards.

* Refactor filter to reduce code repetition.

* Added support for non-Gaussian likelihoods in the EnKF.

* Added support for observing multiple strata.

* Allow unknown params to use wildcards.

* Cleaned up redundant shape calculations.

@JavadocMD JavadocMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See inline.

Comment thread epymorph/forecasting/dynamic_params.py
Comment thread epymorph/forecasting/likelihood.py
Comment thread tests/fast/tools/data_test.py
Comment thread epymorph/forecasting/munge_realizations.py Outdated
Comment thread epymorph/forecasting/munge_realizations.py Outdated
Comment thread epymorph/forecasting/munge_realizations.py Outdated
* Refactor filter code.

* Moved pipeline_messaging

* Typing updates.

* Updated documentation.

* Formatting.

@JavadocMD JavadocMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In addition to the inline comment, I noticed epymorph.forecasting.dynamic_params is importing dataclass from attr instead of dataclasses -- use dataclasses.

There are some documentation warnings, but we don't necessarily have to resolve those here. (uv run mkdocs build to see the list.)

Comment thread epymorph/forecasting/forecast.py Outdated
@JeffreyCovington JeffreyCovington linked an issue Jun 9, 2026 that may be closed by this pull request
@Averydx

Averydx commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

I made some improvements to the typing and handling of parameters to address Tyler's comments.

Comment thread epymorph/forecasting/pipeline.py

@JavadocMD JavadocMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from attr import dataclass

should be: from dataclasses import dataclass

After this (and Jeff's comment), good to merge.

Comment thread epymorph/forecasting/filter_plot.py
Comment thread epymorph/forecasting/filter_plot.py Outdated

@JavadocMD JavadocMD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See inline.

Comment thread epymorph/forecasting/filter_plot.py Outdated
Comment thread epymorph/forecasting/filter_plot.py
@JeffreyCovington JeffreyCovington merged commit 1abed28 into 2.0-beta Jun 30, 2026
1 check passed
@JeffreyCovington JeffreyCovington deleted the Forecasting-Code-Cleanup branch June 30, 2026 19:37
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.

Add a data_mode flag to PipelineOutput

3 participants