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

Allow user to write stuff into iterDict #311

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Allow user to write stuff into iterDict #311

merged 1 commit into from
Oct 24, 2022

Conversation

ewu63
Copy link
Collaborator

@ewu63 ewu63 commented Oct 19, 2022

Purpose

This PR allows the user to add/modify/remove entries in the iterDict which is passed to the user. This offers a second way of adding information to the history file (the first being adding stuff into funcs).

Expected time until merged

A few days

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Checklist

  • I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
  • I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@ewu63 ewu63 requested a review from a team as a code owner October 19, 2022 23:16
@codecov
Copy link

codecov bot commented Oct 19, 2022

Codecov Report

Merging #311 (e95b884) into main (09f4a19) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #311   +/-   ##
=======================================
  Coverage   84.18%   84.19%           
=======================================
  Files          22       22           
  Lines        3332     3334    +2     
=======================================
+ Hits         2805     2807    +2     
  Misses        527      527           
Impacted Files Coverage Δ
pyoptsparse/pySNOPT/pySNOPT.py 89.90% <100.00%> (+0.06%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@marcomangano marcomangano left a comment

Choose a reason for hiding this comment

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

LGTM

@ewu63 ewu63 requested a review from sseraj October 21, 2022 15:59
@ewu63
Copy link
Collaborator Author

ewu63 commented Oct 21, 2022

Hmm not sure what's going on with the Windows build, I just re-triggered it.

@@ -673,6 +673,10 @@ def _snstop(self, ktcond, mjrprtlvl, minimize, n, nncon, nnobj, ns, itn, nmajor,
if not self.storeHistory:
raise Error("snSTOP function handle must be used with storeHistory=True")
iabort = snstop_handle(iterDict)
# write iterDict again if anything was inserted
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully understand this change. Does iterDict get written again or overwritten? Also, does this happen strictly if anything was inserted or are you saying write it again in case anything was inserted?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It gets updated (in the Python dictionary sense) by the new iterDict, then written into the history file. There is no check so this is performed every time. What this mean is that new values/entries are written, but if the user removed an entry or something, the entry remains in the history file.

@sseraj
Copy link
Contributor

sseraj commented Oct 24, 2022

It looks like the Windows failure is unrelated to this change, so I am merging this.

@sseraj sseraj merged commit 03ad440 into main Oct 24, 2022
@sseraj sseraj deleted the update-snstop branch October 24, 2022 14:40
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.

None yet

3 participants