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

1116 add dual variables into the output list in the template #1121

Merged

Conversation

nnhjy
Copy link
Member

@nnhjy nnhjy commented Oct 24, 2024

  1. Add in the current template the dual values as output
  2. Add a dummy parameter to classify the output values

Fixes #1116

Checklist before merging

  • Documentation is up-to-date
  • Unit tests have been added/updated accordingly
  • Code has been formatted according to SpineOpt's style
  • Unit tests pass

@nnhjy nnhjy requested a review from nhniina October 24, 2024 14:35
@nnhjy nnhjy linked an issue Oct 24, 2024 that may be closed by this pull request
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.38%. Comparing base (bb5c565) to head (1659218).
Report is 13 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1121      +/-   ##
==========================================
+ Coverage   87.33%   87.38%   +0.04%     
==========================================
  Files         143      143              
  Lines        4137     4137              
==========================================
+ Hits         3613     3615       +2     
+ Misses        524      522       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nhniina
Copy link
Contributor

nhniina commented Nov 8, 2024

Indentations are sometimes off and we could still discuss with other SpineOpt developers what is the best way to group the outputs. Let's also ensure that an issue is created about the problem with several reports linking to one output, so that the FIXME lines don't get lost. Otherwise I think this looks ready for merging.

align indentation

align indentation

align indentation

align indentation

align indentation
@nnhjy nnhjy force-pushed the 1116-add-dual-variables-into-the-output-list-in-the-template branch from a7b7a36 to ce6078a Compare November 14, 2024 17:57
@nnhjy nnhjy merged commit d894790 into master Nov 15, 2024
7 checks passed
@nnhjy nnhjy deleted the 1116-add-dual-variables-into-the-output-list-in-the-template branch November 15, 2024 10:27
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 dual variables into the output list in the template
2 participants