Skip to content

Allow expressions in Nodes state_initial and state_final parameters#73

Merged
daschw merged 7 commits intoait-energy:mainfrom
daschw:node_expressions
Mar 27, 2025
Merged

Allow expressions in Nodes state_initial and state_final parameters#73
daschw merged 7 commits intoait-energy:mainfrom
daschw:node_expressions

Conversation

@daschw
Copy link
Copy Markdown
Collaborator

@daschw daschw commented Mar 27, 2025

Fix #63
Fix #70

Thanks to Diana for providing the test example.

@daschw
Copy link
Copy Markdown
Collaborator Author

daschw commented Mar 27, 2025

I just saw now that this might conflict with #69 so feel free to ignore and close

Copy link
Copy Markdown
Member

@sstroemer sstroemer 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, thanks a lot!

I did some small changes to use some internal functionality for the isnothing checks - however I'm not sure if that works as is since I only edited it in and couldn't test (sorry).

Comment thread src/core/node.jl Outdated
Comment thread src/core/node/con_first_state.jl Outdated
Comment thread src/core/node/con_last_state.jl Outdated
Comment thread src/core/node/con_last_state.jl Outdated
@sstroemer
Copy link
Copy Markdown
Member

Ah and regarding #69 (which I feel like is just an experimental draft), this one here is definitely the important one and I'd rather get this merged and then figure out the other one. So feel free to merge this one here. We should trigger new releases after that for core and the wrapper right?

@daschw
Copy link
Copy Markdown
Collaborator Author

daschw commented Mar 27, 2025

Any idea why the tests are not running? Did we reach our monthly limit?

use internal functionality for expression checks

Co-authored-by: Stefan Strömer <8915976+sstroemer@users.noreply.github.com>
@sstroemer
Copy link
Copy Markdown
Member

Any idea why the tests are not running? Did we reach our monthly limit?

I fear that is just misconfigured in the workflow. I think these settings do not trigger on adding commits, etc.

@sstroemer
Copy link
Copy Markdown
Member

And the workflow_dispatch only allows manually running it on branches or tags...

@daschw
Copy link
Copy Markdown
Collaborator Author

daschw commented Mar 27, 2025

I see, so I'd have to request a review for the tests to start? Anyway, I'm testing your suggestions locally now.

@sstroemer sstroemer marked this pull request as draft March 27, 2025 13:16
@sstroemer sstroemer marked this pull request as ready for review March 27, 2025 13:16
@sstroemer
Copy link
Copy Markdown
Member

I see, so I'd have to request a review for the tests to start? Anyway, I'm testing your suggestions locally now.

Yes, seems to work - maybe a bit of a workaround, but maybe it's not that bad of an idea overall (I initially disabled the "on push" trigger because it would otherwise re-run the tests for every commit that you add, which could be more often then we'd like to).

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.

Project coverage is 48.81%. Comparing base (463763b) to head (7a4f3b0).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/core/node.jl 66.66% 3 Missing ⚠️
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   48.28%   48.81%   +0.53%     
==========================================
  Files         111      112       +1     
  Lines        4548     4560      +12     
==========================================
+ Hits         2196     2226      +30     
+ Misses       2352     2334      -18     
Files with missing lines Coverage Δ
src/core/node/con_first_state.jl 100.00% <100.00%> (ø)
src/core/node/con_last_state.jl 88.88% <100.00%> (+7.40%) ⬆️
src/core/node/var_state.jl 100.00% <100.00%> (ø)
src/parser.jl 83.45% <100.00%> (+1.60%) ⬆️
src/core/node.jl 58.10% <66.66%> (+1.39%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@daschw daschw merged commit 0011418 into ait-energy:main Mar 27, 2025
5 checks passed
daschw added a commit to daschw/IESopt.jl that referenced this pull request May 7, 2025
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.

Refactor Node.state_initial to an Expression Allow state_cyclic together with state_ final

2 participants