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

Multi agent calibration with pyswarm PSO integration. #41

Merged
merged 54 commits into from
Jul 18, 2023

Conversation

hellkite500
Copy link
Member

Support for PSO and potentially other multi-agent optimization algorithms.

See changelog for details.

Testing

  1. Not as much test coverage as I would like, but tests updated and a couple new ones added...all passing locally

Todos

  • More test coverage.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Linux
  • MacOS

@hellkite500 hellkite500 force-pushed the multi-agent-calibration branch 12 times, most recently from 02b4648 to 75d7672 Compare December 9, 2022 21:05
@hellkite500 hellkite500 linked an issue Dec 9, 2022 that may be closed by this pull request
Copy link
Contributor

@mattw-nws mattw-nws left a comment

Choose a reason for hiding this comment

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

Probably not finished here, but some things for interim consideration.

python/ngen_cal/src/ngen/cal/strategy.py Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/objectives.py Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/objectives.py Show resolved Hide resolved
@aaraney
Copy link
Member

aaraney commented Feb 16, 2023

Sorry, @hellkite500, for whatever reason this has slipped through my feed and i've not reviewed it. Im stepping away from the keyboard for an hour or so this morning, but i'll give it a review once im back.

Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

Mostly small stuff, but there are a few comments that will need to be a few changes before this can get merged.

.github/workflows/ngen-cal.yaml Outdated Show resolved Hide resolved
.github/workflows/ngen-conf.yml Outdated Show resolved Hide resolved
python/calibration.py Outdated Show resolved Hide resolved
python/ngen_cal/changelog.md Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/__main__.py Outdated Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/ngen.py Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/ngen.py Outdated Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/ngen.py Outdated Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/search.py Outdated Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/utils.py Show resolved Hide resolved
@hellkite500
Copy link
Member Author

Remove the test deduplication parts into its own PR.

@hellkite500 hellkite500 force-pushed the multi-agent-calibration branch 4 times, most recently from a8ec261 to 82d9e48 Compare July 17, 2023 22:46
Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

Overall it looks great! Thanks for making the requested changes. There are two minor logical statements that I had questions about, but after we get through them, this should be ready to merge.

python/ngen_cal/src/ngen/cal/model.py Outdated Show resolved Hide resolved
python/ngen_cal/src/ngen/cal/model.py Outdated Show resolved Hide resolved
aaraney
aaraney previously approved these changes Jul 18, 2023
Copy link
Member

@aaraney aaraney left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@hellkite500 hellkite500 dismissed mattw-nws’s stale review July 18, 2023 18:34

Addressed one major request, and opened an issue for the other. This PR is blocking some others that need review and merged. Feel free to test out and leave feed back on the linked issue or in a new one as needed!

@hellkite500 hellkite500 merged commit f846441 into NOAA-OWP:master Jul 18, 2023
11 checks passed
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.

Duplicated github actions?
3 participants