Skip to content

Warn about non-idempotent values in MFILE#3351

Merged
jonmaddock merged 5 commits intomainfrom
3249-warn-rather-than-raise-exception-when-idempotence-check-fails
Oct 31, 2024
Merged

Warn about non-idempotent values in MFILE#3351
jonmaddock merged 5 commits intomainfrom
3249-warn-rather-than-raise-exception-when-idempotence-check-fails

Conversation

@jonmaddock
Copy link
Copy Markdown
Contributor

Warn, rather than raise exception, when MFILE variables are non-idempotent (still varying after 10 model evaluations). The variables and their values are put into a table in stdout.

@jonmaddock jonmaddock linked an issue Oct 17, 2024 that may be closed by this pull request
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 13.79310% with 25 lines in your changes missing coverage. Please review.

Project coverage is 27.98%. Comparing base (b68ac63) to head (93aeb61).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
process/caller.py 8.33% 22 Missing ⚠️
process/final.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3351      +/-   ##
==========================================
+ Coverage   27.11%   27.98%   +0.86%     
==========================================
  Files          76       76              
  Lines       17736    18881    +1145     
==========================================
+ Hits         4810     5284     +474     
- Misses      12926    13597     +671     

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

@jonmaddock
Copy link
Copy Markdown
Contributor Author

@j-a-foster @chris-ashe can you check that you can't reproduce this problem any more? I can't seem to reproduce it, but I've imitated it to test this solution.

@j-a-foster
Copy link
Copy Markdown
Collaborator

j-a-foster commented Oct 21, 2024

I've tested two input files on this branch, that fail on main, and they now pass. I can't see the warning in the OUT.DAT though?

Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

I agree with @j-a-foster that putting this warning and the table in the OUT.DAT (and maybe the MFILE.DAT) could be quite a good idea too.

Comment thread process/caller.py Outdated
Comment thread process/caller.py Outdated
Comment thread process/caller.py Outdated
@jonmaddock
Copy link
Copy Markdown
Contributor Author

I've tested two input files on this branch, that fail on main, and they now pass. I can't see the warning in the OUT.DAT though?
Here's the new OUT.DAT output.

# ^Numerics 
   26  TF coil conduit stress upper lim      <  7.5000E+08 Pa           1.1159E+08 Pa        
 
 ****************************************** NON-IDEMPOTENT VARIABLES ******************************************
 
 Model evaluations at the current optimisation parameter vector don't produce idempotent values in the final output.
Variable      Previous value    Current value
----------  ----------------  ---------------
rmajor                     1                8
 
 *************************************** Power Reactor Costs (1990 US$) ***************************************

I've decided not to put anything in the MFILE output, because I don't think it's the right place for error messages.

Copy link
Copy Markdown
Collaborator

@timothy-nunn timothy-nunn left a comment

Choose a reason for hiding this comment

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

Happy with the changes. Curious why there is a warn and print statement each handling half of the out. Maybe consider changing this if its not necessary.

Comment thread process/caller.py Outdated
Comment on lines +194 to +195
warnings.warn("\033[93m" + non_idempotent_warning)
print(non_idempotent_table + "\033[0m")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is all of the output not handled by the warn function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Quite right, I've combined these.

@jonmaddock jonmaddock merged commit 266891f into main Oct 31, 2024
@jonmaddock jonmaddock deleted the 3249-warn-rather-than-raise-exception-when-idempotence-check-fails branch October 31, 2024 15:41
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.

Warn rather than raise exception when idempotence check fails

4 participants