stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
138 stars 44 forks source link

Update js_of_ocaml to 4.1.0 #1365

Closed WardBrian closed 9 months ago

WardBrian commented 9 months ago

This bumps us from 3.11 to 4.1, which includes a fix for an inlining issue that can affect performance that @andrjohns found discussions of. Some quick local tests suggested that this is about 10% faster just from updating.

@serban-nicusor-toptal - I think to actually get this going in our CI we need to re-build our docker images, right?

Submission Checklist

Release notes

Updated the OCaml-to-JS compiler used to create stanc.js

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

serban-nicusor-toptal commented 9 months ago

Hey @WardBrian that's correct, I'll build them later when I get home.

WardBrian commented 9 months ago

Thanks! I believe the only image which matters in this case is debianfi, since it is only the JS build that changed

serban-nicusor-toptal commented 9 months ago

I've pushed two new images:

WardBrian commented 9 months ago

@serban-nicusor-toptal after switching the images getting a CI failure dune: not found

serban-nicusor-toptal commented 9 months ago

That must be because of the opam install I think, I'll rebuild and test locally then push a new one.

serban-nicusor-toptal commented 9 months ago

@WardBrian everything should be working fine now!

WardBrian commented 9 months ago

Great, thanks!

Do we rename the image to debian or should we just merge with the Jenkinsfile changed?

serban-nicusor-toptal commented 9 months ago

I think it's fine to leave it as is, I've used debianfi after the migration to Flatiron CI. We also have the latest state in ci-scripts repo.

WardBrian commented 9 months ago

Great, thanks!

I’m considering another update soon (to OCaml 4.14, the last of the 4.x releases) so we will probably change the images again at that point