Hi @dutta-alankar —
I found some time to look at this some more today, and am now even more confident that this represents a bug in our implementation rather than in your code. I don't know what the fix is on our side yet, but I managed to boil one of your errors down to a very simple reproducer and opened an issue for it here: [Bug]: domain may be used before it is initialized via (unused) record's fields · Issue #27453 · chapel-lang/chapel · GitHub
I also followed up on Lydia's suggestion to create a warning for Stencil-distributed arrays of owned class types to help future users avoid the confusion that we hit at the top of this discussion: Generate a warning (or two?) for stencil-distributed arrays of owned … by bradcray · Pull Request #27454 · chapel-lang/chapel · GitHub
Catching up on some of your questions that I missed earlier this week:
So as a workaround, I created a dummy domain variable inside the record called
states_countand made it equal tostate_domain. I do not have any understanding of why this was needed.
This puzzling behavior is related to the bug I filed above—and you are correct in being puzzled, as neither change ought to be needed, we're just finding ways to work around this error message.
As noted on the issue I opened above, one puzzling behavior is that if I change the declaration of state_domain from const state_domain: domain(1) = {1..5}; to simply const state_domain = {1..5};, then it seems to work fine. This may also explain why this bug is unfamiliar to me despite the pattern not being very strange or unusual. In practice, I think we typically rely on Chapel's type inference rather than explicitly declaring the types of things, so may have been unlikely to hit patterns like this in our own code (though I'm still somewhat surprised other users haven't… It feels like people are about 50/50 between whether they use type declarations or not).
- This entire cure seems quite of a magic to me, and I have no understanding of why this is needed and what was going wrong in the earlier code.
This one I didn't manage to get back to today, but I have a feeling it is related to the previous bug. I just haven't been able to reduce it to a simple reproducer yet.
which I perhaps understand as it needs a default
initwith no arguments (Is it correct to interpret the error like this? I thought that records always come with a default initializer and I didn't have to create one manually).
Your interpretation is generally correct, you're just missing one element. Chapel will generate a default initializer if you don't declare any initializers for your record/class, but as soon as you add one, it replaces the compiler-generated initializer (the rationale being an assumption that the type author would not want such an initializer for their type if they've specified one). We've discussed adding a mechanism to permit a type author to say "despite the fact that I've created an initializer, please give me the compiler-generated one as well," but have not yet implemented such a feature.
Note that you could combine your two initializers into one by using a default argument like this:
proc init (position: real(64) = 0.0): void {
this.position = position;
this.states_count = state_domain;
this.state_prims = [i in this.states_count] 0.0;
this.state_consv = [i in this.states_count] 0.0;
this.dummy_initialized = false;
}
This would allow you to pass in a value, or not. Since this could be called with no arguments, it would support the ability to write var wall_left: wall;
In addition, since real arrays default to 0.0 and boolean variables/fields default to false, you could simply write it as:
proc init (position: real(64) = 0.0): void {
this.position = position;
this.states_count = state_domain;
}
since the compiler will default-initialize any fields that you don't.
Finally, note that while the compiler-generated initializer is able to be called with no arguments, you can also pass an argument into it per field. So if you were to declare your record as:
record wall {
var position: real(64);
var states_count: domain(1) = state_domain; // or `= {1..5};`
var state_prims: [states_count] real(64);
var state_consv: [states_count] real(64);
var dummy_initialized: bool;
}
the compiler-generated initializer for wall would effectively have the signature:
proc init(position: real(64) = 0.0,
states_count: domain(1) = state_domain, // or `= {1..5}`
state_prims: [states_count] real(64) = [i in states_count] 0.0,
state_consv: [states_count] real(64) = [i in states_count] 0.0,
dummy_initialized: bool = true) {
// and then it would assign each field its respective argument…
}
and this would permit you to declare no initializers while still writing both of your styles of calls to new wall(…). For this type, the main reason you might choose to create an initializer like the ones above, or the ones you did, would be if you wanted to prevent users of your type from passing in values for the other fields. If you control all the call sites because you're the only user, then sticking with the compiler-generated initializer is often most productive.
I believe can potentially evolve to be a very useful HPC tool -- simple yet powerful fluid dynamic solver.
Let us know if you'd like to do code reviews on your program as you make progress with it. I think there are other things that could be streamlined or simplified (e.g., there's no reason to put a sync around your forall loops), but this message has gone on long enough already, and it may be more time-efficient and instructive to do interactively. You could propose this as an activity for one of our Thursday deep-dive slots, for example. Personally, I'll be out of the office for the next few weeks, so wouldn't be able to be involved for awhile. But I suspect others would step up while I'm away if you were interested.
-Brad