Skip to content

Housekeeping#199

Merged
klamike merged 33 commits into
mainfrom
mk/refactordeps
Jan 25, 2026
Merged

Housekeeping#199
klamike merged 33 commits into
mainfrom
mk/refactordeps

Conversation

@klamike

@klamike klamike commented Aug 2, 2025

Copy link
Copy Markdown
Contributor

This is ready, except I need to check the slurm pipeline works on phoenix.

  • MathOptSymbolicAD -> MOI.Nonlinear.SymbolicMode
  • CI for latest and for LTS
  • Use julia-actions/cache@v2
  • Default to 8 cores for sysimage
  • switch time_limit kwarg default to Inf in exp/sampler.jl, check if existing time limit matches for less verbose warnings when not using time limits

Breaking changes:

  • slurm/submit_jobs.jl -> slurm/create_jobs.jl (since it doesn't actually submit anymore)
  • close On HSL_jll and OTHER_ERROR #200 . we now warn and skip if HSL_jll is a dummy
  • Use user's julia instead of lmod's. To use the old workflow, add julia to the module load list in slurm/template/env.sh

Comment thread test/sampler.jl Outdated
@codecov

codecov Bot commented Aug 2, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/opf/ed.jl 95.38% <100.00%> (+0.03%) ⬆️
src/opf/opf.jl 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread exp/Project.toml Outdated
@klamike klamike linked an issue Aug 3, 2025 that may be closed by this pull request
Comment thread slurm/make_sysimage.jl Outdated
@klamike klamike marked this pull request as ready for review August 15, 2025 18:24
@klamike klamike requested a review from mtanneau August 15, 2025 18:24
@klamike

klamike commented Nov 24, 2025

Copy link
Copy Markdown
Contributor Author

@mtanneau what do you think of this split project approach?

@mtanneau

Copy link
Copy Markdown
Contributor

Good with all of these:

  • MathOptSymbolicAD -> MOI.Nonlinear.SymbolicMode
  • CI for latest and for LTS
  • Use julia-actions/cache@v2
  • Default to 8 cores for sysimage
  • slurm/submit_jobs.jl -> slurm/create_jobs.jl (since it doesn't actually submit anymore)

But TBH I have mixed feelings about this one:

  • Separate projects for root, exp, and slurm

🙂 I am supportive of reducing / cleaning up our dependencies, and the fact that our codebase is effectively scatter across src, exp and slurm.
🙁 However, it does scare me a little to have 3 different environments that can possibly (likely) fall out of sync with one another.

I would rather we consolidate all these pieces into src and leverage the app functionalities introduced in Julia 1.12... which I believe would also reduce our CI burden quite a bit. That is a non-trivial amount of work, and I would be 100% open to splitting it into a separate PR (or PRs)

@mtanneau

Copy link
Copy Markdown
Contributor

CI is failing, looks like it's because of an issue with Clarabel.

Comment thread .github/workflows/ci.yml Outdated
@klamike

klamike commented Nov 24, 2025

Copy link
Copy Markdown
Contributor Author

I'll rebase and revert to a single Project.toml. The main motivation is to avoid pulling different versions of dependencies across src, exp, slurm.

Copilot AI review requested due to automatic review settings December 17, 2025 22:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request reorganizes the project's dependency structure by introducing separate Julia project environments for different use cases (root, exp, and slurm), modernizes the automatic differentiation backend by replacing MathOptSymbolicAD with MOI.Nonlinear.SymbolicMode, and improves CI configuration.

Key Changes:

  • Separated dependencies into three project environments: root for core PGLearn functionality, exp/ for experimentation/sampling scripts, and slurm/ for cluster job management
  • Replaced deprecated MathOptSymbolicAD with MOI.Nonlinear.SymbolicMode via a new maybe_set_ad() helper function
  • Updated CI to test on both latest Julia and LTS versions, and modernized caching with julia-actions/cache@v2

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Project.toml Removed solver-specific and utility dependencies now moved to exp/ and slurm/ projects
exp/Project.toml New project file for experimentation environment with solvers and sampling dependencies
slurm/Project.toml New project file for SLURM job management with templating and I/O dependencies
src/opf/opf.jl Removed MathOptSymbolicAD import, added maybe_set_ad() helper using MOI.Nonlinear.SymbolicMode
src/opf/ed.jl Updated to use new AD backend via maybe_set_ad()
exp/sampler.jl Removed obsolete MathOptSymbolicAD import
exp/config.toml Updated example configuration with placeholder values for SLURM settings
slurm/create_jobs.jl Renamed from submit_jobs.jl, added error handling for incorrect project usage
slurm/make_sysimage.jl Updated package list and project paths to use exp/ environment
slurm/template/*.sbatch Updated all SLURM templates to use correct project paths (exp/ or slurm/)
slurm/README.md Updated documentation for file rename and new project structure with setup instructions
README.md Added setup instructions for new project structure
test/sampler.jl Added environment setup logic with version-specific handling for Julia ≤1.10
.github/workflows/ci.yml Added LTS testing, replaced custom caching with julia-actions/cache@v2

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yml
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@klamike

klamike commented Dec 17, 2025

Copy link
Copy Markdown
Contributor Author

@copilot add a commit to go back to a single Project.toml file

Copilot AI commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

@klamike I've opened a new pull request, #214, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan

* Consolidate to single unified Project.toml

Co-authored-by: klamike <17013474+klamike@users.noreply.github.com>

* Update all references to use single unified project

Co-authored-by: klamike <17013474+klamike@users.noreply.github.com>

* Remove unnecessary try/catch for Mustache in create_jobs.jl

Co-authored-by: klamike <17013474+klamike@users.noreply.github.com>

* Update comment in make_sysimage.jl to use --project=.

Co-authored-by: klamike <17013474+klamike@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: klamike <17013474+klamike@users.noreply.github.com>
Comment thread slurm/make_sysimage.jl

Copilot AI commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

@klamike I've opened a new pull request, #216, to work on those changes. Once the pull request is ready, I'll request review from you.

Comment thread test/sampler.jl Outdated
@klamike klamike changed the title Organize dependencies Housekeeping Dec 18, 2025
@ivanightingale ivanightingale self-requested a review January 23, 2026 22:55
Comment thread test/utils/io.jl Outdated
@klamike klamike merged commit cbb563d into main Jan 25, 2026
4 checks passed
@klamike klamike deleted the mk/refactordeps branch January 25, 2026 00:38
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.

On HSL_jll and OTHER_ERROR Reduce dependencies

5 participants