Skip to content

Conversation

adowling2
Copy link
Contributor

@adowling2 adowling2 commented Mar 10, 2025

Fixes

Summary/Motivation:

  • If the IDS problem is infeasible, it means the constraint is not a major component of an IDS
  • Logic was incorrect

Changes proposed in this PR:

  • Replaces a ValueError with an appropriate print statement.
  • Slight improvement to printing in the SVDToolbox

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@adowling2
Copy link
Contributor Author

Sorry, I was not careful with my branches. This also includes the simple edit in #1590

@adowling2
Copy link
Contributor Author

This is ready to review.

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Mar 13, 2025
Copy link
Contributor

@bpaul4 bpaul4 left a comment

Choose a reason for hiding this comment

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

Hi @adowling2, thanks for opening this. To resolve the test failures, I believe you just need to update the expected string object. Try adding print(stream.get_value()) right before the failing assert check, it should print the correct string for you in the error log, then update expected and remove the print statement.

Copy link
Contributor

@dallan-keylogic dallan-keylogic left a comment

Choose a reason for hiding this comment

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

The changes here look good, the tests just need to be updated with the new output values.

)

stream.write(f"{TAB}Smallest Singular Value {e}:\n\n")
stream.write(f"{TAB}Smallest Singular Value {e} ({self.s[e-1]:.3e}):\n\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for this change---I was going to make it myself at some point.

f"constraint {cons.name}. This probably indicates the constraint"
" is not a major component of an IDS."
)
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

return None works just like return. Since this statement is at the end of the function, no return statement is needed at all. You don't need to actually change anything here (unless Pylint says to), but I thought I'd point it out.

@ksbeattie
Copy link
Member

@adowling2, any progress on this (given the above suggestions)?

@dallan-keylogic
Copy link
Contributor

@adowling2 Will you have the chance to address these comments anytime soon?

@adowling2
Copy link
Contributor Author

This is back on my radar now that the semester is over.

@ksbeattie
Copy link
Member

@adowling2 is this now back off your radar?

@ksbeattie
Copy link
Member

@adowling2, now moving to Nov...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants