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.

Sign up for GitHub

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

Jump to bottom

Non-deterministic code generation #2241

Closed
dweindl opened this issue Dec 15, 2023 · 2 comments · Fixed by #2242
Closed

Non-deterministic code generation #2241

dweindl opened this issue Dec 15, 2023 · 2 comments · Fixed by #2242
Assignees
@dweindl
Labels
python

Comments

@dweindl
Copy link
Member

dweindl commented Dec 15, 2023

For my current model, the order of state variables changes between subsequent imports.
Not a big issue, but something we'd like to avoid.
@dweindl dweindl added the python label Dec 15, 2023
@dweindl
Copy link
Member Author

dweindl commented Dec 15, 2023 •
edited
Loading

Possible that the change was related to different toposort after #2234 .
Couldn't reproduce with the same current amici version. Closing.

@dweindl dweindl closed this as completed Dec 15, 2023
@dweindl
Copy link
Member Author

dweindl commented Dec 15, 2023

Found it. Processing of event assignments.

@dweindl dweindl reopened this Dec 15, 2023
@dweindl dweindl self-assigned this Dec 15, 2023
dweindl added a commit to dweindl/AMICI that referenced this issue Dec 15, 2023
@dweindl
Deterministic order of event assignments
4b117d9 
Ensure event assignments targets are processed in deterministic order.
Otherwise the ordering of state variables may change between subsequent
model imports, which we'd like to avoid.

Closes AMICI-dev#2241.
@dweindl dweindl mentioned this issue Dec 15, 2023
Merged
@dweindl dweindl linked a pull request Dec 15, 2023 that will close this issue
Deterministic order of event assignments #2242
Merged
dweindl added a commit to dweindl/AMICI that referenced this issue Dec 18, 2023
@dweindl
Deterministic order of event assignments (AMICI-dev#2242)
bc254c8 
Ensure event assignments targets are processed in deterministic order.
Otherwise the ordering of state variables may change between subsequent
model imports, which we'd like to avoid.

Closes AMICI-dev#2241.
dweindl added a commit that referenced this issue Dec 18, 2023
@dweindl @dilpath
code-gen: Simplify switch (#2240)
0cc43f9 
Improves model code generation by collapsing cases with identical statements.

I.e.
```
switch(a)
 case b:
 case c:
  statements;
  break;
```
instead of

```
switch(a)
 case b:
  statements;
  break;
 case c:
  statements;
  break;
```

For my current model of interest, containing many events, this significantly reduces the generated code:

E.g.:
```
 16K my_model/deltasx.cpp
6,6M my_model_old/deltasx.cpp
```

Overall, for this model, I got from 204201 LOC down to 7936 LOC (i.e. -96%).

* ..

* Apply suggestions from code review

Co-authored-by: Dilan Pathirana <[email protected]>

* GHA: test python3.12 (#2179)

Run nightly tests also on python3.12

* Deterministic order of event assignments (#2242)

Ensure event assignments targets are processed in deterministic order.
Otherwise the ordering of state variables may change between subsequent
model imports, which we'd like to avoid.

Closes #2241.

* Fix AMICI hiding all warnings (#2243)

Previously, importing amici would result in all warnings of the program being hidden, due to `logging.captureWarnings(True)`:

```sh
$ python -c "import warnings; warnings.warn('bla');"
<string>:1: UserWarning: bla
$ python -c "import amici; import warnings; warnings.warn('bla');"
$
```

This can't be the desired default.

Changes:
* Default to not capturing warnings
* If warnings are to be captured, at least handle them by amici loggers

Closes ICB-DCM/pyPESTO#1252


---------

Co-authored-by: Dilan Pathirana <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dweindl dweindl

Labels
python
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deterministic order of event assignments dweindl/AMICI
1 participant
@dweindl