Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 30, 2025

This PR fixes critical issues in the optimization exercises in class01/background_materials/optimization_basics.jl that were preventing proper functionality in Pluto.jl notebooks.

Issues Fixed

1. Sudoku Exercise Answer Checking (Issue #32)

Tuple Syntax Error: The ground_truth_s definition was missing a trailing comma, causing it to create a Vector instead of a NamedTuple:

# Before (creates a Vector)
ground_truth_s = (x_ss = [[matrix]])

# After (creates a proper NamedTuple)  
ground_truth_s = (x_ss = [[matrix]],)

Incompatible Matrix Format Comparison: The answer checking was attempting to directly compare incompatible matrix formats:

  • anss.x_ss: A 3D binary matrix from JuMP.value.(sudoku[:x_s]) where x_s[i,j,k] = 1 means value k is at position (i,j)
  • ground_truth_s.x_ss: A 2D solution matrix with sudoku values 1-9

Solution: Added a conversion function convert_3d_to_solution() that transforms the 3D binary indicator matrix into the standard 2D sudoku solution format for proper comparison.

2. Variable Naming Conflicts (Issue #31)

Fixed a Pluto.jl variable naming conflict in the Rosenbrock valley exercise where line 726 incorrectly referenced a bare x variable:

# Before (causes Pluto.jl conflict)
elseif x == 25.0

# After (correctly references student answer)
elseif ansd == 25.0

This was causing conflicts because x referenced the JuMP variable from the NLP model instead of the student's answer ansd, creating variable dependency issues in Pluto.jl.

Verification

  • Both fixes maintain backward compatibility
  • File parses successfully without syntax errors
  • No other variable naming conflicts identified
  • Changes are minimal and surgical

Fixes #32 and #31.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Contributor Author

Copilot AI commented Aug 30, 2025

@andrewrosemberg 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI and others added 2 commits August 30, 2025 01:20
Co-authored-by: andrewrosemberg <24196241+andrewrosemberg@users.noreply.github.com>
Co-authored-by: andrewrosemberg <24196241+andrewrosemberg@users.noreply.github.com>
Copilot AI changed the title [WIP] Answer checking of sudoku exercise Fix sudoku exercise answer checking in optimization_basics.jl Aug 30, 2025
Copilot AI requested a review from andrewrosemberg August 30, 2025 01:22
@klamike
Copy link
Collaborator

klamike commented Aug 31, 2025

Did you trigger this somehow @andrewrosemberg ? Or it will just randomly create its own bug fix PRs?

@andrewrosemberg
Copy link
Member

Did you trigger this somehow @andrewrosemberg ? Or it will just randomly create its own bug fix PRs?

trigger in the issue

@andrewrosemberg
Copy link
Member

@copilot can you also propose a fix for #31 in this PR!

Co-authored-by: andrewrosemberg <24196241+andrewrosemberg@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 2, 2025

@copilot can you also propose a fix for #31 in this PR!

I've added the fix for issue #31 to this PR! The problem was a variable naming conflict in line 726 where x == 25.0 should have been ansd == 25.0. This was causing Pluto.jl conflicts by referencing a bare x variable instead of the student's answer. Fixed in commit 4784231.

Copilot AI changed the title Fix sudoku exercise answer checking in optimization_basics.jl Fix sudoku exercise answer checking and variable naming conflicts in optimization_basics.jl Sep 2, 2025
@andrewrosemberg
Copy link
Member

@ivanightingale might checking if these solved your problem? It seems that it now works for me

@andrewrosemberg andrewrosemberg marked this pull request as ready for review September 2, 2025 23:44
@andrewrosemberg andrewrosemberg merged commit b41df23 into main Sep 3, 2025
1 check passed
@andrewrosemberg andrewrosemberg deleted the copilot/fix-32 branch September 30, 2025 16:26
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.

Answer checking of sudoku exercise

3 participants