Skip to content

Conversation

WalterMadelim
Copy link
Contributor

@WalterMadelim WalterMadelim commented Sep 18, 2025

The original description is not comprehensive. I add a quote from Julia's doc.

PS some related discussion https://discourse.julialang.org/t/is-it-defined-behavior-to-modify-iter-while-for-looping-over-it/132477/14?u=waltermadelim

The original description is not comprehensive. I add a quote from Julia's doc.
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1535ecb) to head (2765723).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #4072   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           43        43           
  Lines         6203      6203           
=========================================
  Hits          6203      6203           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Clarify the explanation of race conditions and thread safety.
@odow
Copy link
Member

odow commented Sep 18, 2025

I made a tweak. Any better?

@WalterMadelim
Copy link
Contributor Author

"mutate" should be a sharper word than "modify"? describing functions like circshift!(v), which has a ! suffix and in-place modifying its argument.

PS I'm currently very confused about the "name" (variable) in julia, whether they refer to the same object. As that link suggests. I'll figure that out first.

Clarified the explanation of data races and provided examples of incorrect multi-threading usage with JuMP models.
@odow
Copy link
Member

odow commented Sep 19, 2025

What about this. I added a much larger description and example

@WalterMadelim
Copy link
Contributor Author

WalterMadelim commented Sep 19, 2025

I think "Read and write x" sounds better than "Read and write x[]" (a minor issue).
Others are okay.

type 1 modification is to mutate something
circshift!(x, 1)

type 2 modification is reuse the name and re-bind it to another object
x = circshift(x, 1)

type 3 modification

let
    f = () -> println(x)
    x = 1
    f()
    x = "s" # here `f` is inadvertently modified!!
    f()
end
let
    c = [1]
    x = [c]
    c[] += 1 # here `x` is inadvertently modified
    x
end

(Any others?)

@odow
Copy link
Member

odow commented Sep 20, 2025

Type 1: you're modifying x. Type 2: whether you modify x of another thread depends on the scope of x = . Type 3: this is "type 1".

If I can give you some unsolicited advice, your questions on Discourse all stem from the fact there Julia's scoping has some tricky edge cases. So just don't write code like that. Don't do your Type 3 example where x is referenced in f before it is created. Sure, you can get it to work, but just please don't. The code is hard to read, and there's no need to be complicated. If you want something mutable, don't rely on scope re-binding names etc; explicitly use a mutable container.

@WalterMadelim
Copy link
Contributor Author

Don't do your Type 3 example where x is referenced in f before it is created. Sure, you can get it to work, but just please don't. The code is hard to read, and there's no need to be complicated

I think there is some philosophy in there for julia to deem it valid. The unintroduced x inside f can be seen as a place holder. Besides, the f is only created there, it is not called. For a function, it makes contribution only if it is called. And at this time it looks for the latest object that x refers to---this behavior is exactly what I want when I'm writing that asynchronous master-subproblems problem---a subproblem always want to generate cut based on the latest trial solution generated by the master.

Using a mutable container is also convincing, I'll rethink that.

@odow
Copy link
Member

odow commented Sep 20, 2025

I think there is some philosophy in there for julia to deem it valid

Just because it can be valid doesn't mean that you should use it.

And at this time it looks for the latest object that x refers to---this behavior is exactly what I want when I'm writing that asynchronous master-subproblems problem---a subproblem always want to generate cut based on the latest trial solution generated by the master.

I strongly disagree with this approach. You should explicitly pass the master solution to the subproblem. Don't rely on scope changing. Push first-stage primal solutions to a channel or a vector. Don't rely on scoping rules to change the value of x.

Anyway, we're off-topic for this PR, which was useful and appreciated 😄

@odow odow merged commit a0ed4c6 into jump-dev:master Sep 20, 2025
10 checks passed
@WalterMadelim
Copy link
Contributor Author

use a mutable container

It indeed appears to be a bit cleaner under the lens of code_warntype

function masterˈs_loop_rebinding_version(snap)
    while true
        # optimize!(master_model)
        snap = (x = value(x), z = round(Int, value(z)))
        for j = 1:4 # assume 4 blocks
            Threads.@spawn subproblemˈs_duty(j, snap)
        end
    end
end
@code_warntype masterˈs_loop_rebinding_version((x = 3.4, z = 2))

const snap = NamedTuple[(x = 3.4, z = 2)]
function masterˈs_loop_container_version()
    while true
        # optimize!(master_model)
        snap[] = (x = value(x), z = round(Int, value(z)))
        for j = 1:4 # assume 4 blocks
            Threads.@spawn subproblemˈs_duty(j, snap[])
        end
    end
end
@code_warntype masterˈs_loop_container_version()

@WalterMadelim
Copy link
Contributor Author

Share some further thoughts about the issue. https://discourse.julialang.org/t/is-it-defined-behavior-to-modify-iter-while-for-looping-over-it/132477/33?u=waltermadelim

Anyway, The async programming with @spawn is really a dilemma:

  1. if you use name capturing, you concern the correctness of Core.setfield!
  2. if you use mutable container, you have to add lots of locks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants