Skip to content

WIP: check update#88

Merged
jbcaillau merged 23 commits intomainfrom
abstract_checks
May 30, 2023
Merged

WIP: check update#88
jbcaillau merged 23 commits intomainfrom
abstract_checks

Conversation

@jbcaillau
Copy link
Copy Markdown
Member

@ocots @joseph-gergaud Hi! The name of the branch is a bit misleading as there are updates in checks in abstract and functional code. Several points:

  • variable, time, state, control, dynamics and objective must be unique (= 0 or 1) declarations. simpler and sounder.
  • @joseph-gergaud I was intending to forbid sth like
@def o begin
       t  [ 0, 1 ], time
       x  R, state
       u  R, control
       x(0) + x(2) == 0, (1)
       end

but it is correct julia (and ct) syntax, so for the moment I prefer not to add a more defensive check. (If there is a function named x in the user env - a bad idea, indeed - this will call it when evaluating the boundary condition.)

  • @ocots do we agree that the current code checks that variable (if NonFixed), time, state and control are checked to be declared before constraint!, dynamics! and objective! are called? this is not minimal (e.g., to declare a pure state constraint one does not need to declare the control...) but, again, simpler and sounder.
  • @ocots looks like @__check is not really a macro (and a macro must not throw and exception - instead generate an expression that does...) trying to update it, then PR-ing.

@github-actions github-actions bot requested a review from ocots May 25, 2023 14:36
@codecov
Copy link
Copy Markdown

codecov bot commented May 25, 2023

Codecov Report

Merging #88 (6f5ee78) into main (8f661a2) will decrease coverage by 0.90%.
The diff coverage is 96.96%.

❗ Current head 6f5ee78 differs from pull request most recent head 9281385. Consider uploading reports for the commit 9281385 to get more accurate results

@@            Coverage Diff             @@
##             main      #88      +/-   ##
==========================================
- Coverage   63.39%   62.50%   -0.90%     
==========================================
  Files          16       16              
  Lines        2330     2272      -58     
==========================================
- Hits         1477     1420      -57     
+ Misses        853      852       -1     
Impacted Files Coverage Δ
src/repl.jl 0.00% <0.00%> (ø)
src/checking.jl 100.00% <100.00%> (+3.03%) ⬆️
src/ctparser_utils.jl 100.00% <100.00%> (ø)
src/model.jl 96.80% <100.00%> (+0.73%) ⬆️
src/onepass.jl 99.63% <100.00%> (-0.07%) ⬇️
src/types.jl 77.77% <100.00%> (+1.65%) ⬆️

@jbcaillau
Copy link
Copy Markdown
Member Author

@ocots not sure about what you want to do here, so I'm leaving the macro

  • @ocots looks like @__check is not really a macro (and a macro must not throw and exception - instead generate an expression that does...) trying to update it, then PR-ing.

Please review and merge!

@ocots
Copy link
Copy Markdown
Member

ocots commented May 30, 2023

Indeed, for constraint!, objective! and dynamics!, there is a first check:

@__check(ocp)

that checks if state, control, time and variable are set. This will have to be updated when we will consider to add box constraints within the declaration of dimensions.

@jbcaillau jbcaillau merged commit 0407303 into main May 30, 2023
@ocots
Copy link
Copy Markdown
Member

ocots commented May 30, 2023

I think I can remove the macro. I try.

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.

2 participants