StudioInfinity/dyna3
2
0
Fork 1
Code Issues 44 Pull requests Projects 2 Releases 2 Wiki Activity Actions

Generalize constraints to observables #48

Merged
glen merged 25 commits from Vectornaut/dyna3:observables_on_main into main 2025-03-10 23:43:25 +00:00
Conversation 69 Commits 25 Files changed 7 +6 -9
1 changed files with 6 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 8b4a72c60c - Show all commits

15
app-proto/src/assembly.rs
View file

@ -292,15 +292,12 @@ impl Assembly {
self.regulators.with_untracked(|regs| {
for (_, reg) in regs {
reg.set_point.with_untracked(|set_pt| {
match set_pt {
Absent => (),
Present { value, .. } => {
let subjects = reg.subjects;
let row = elts[subjects.0].column_index.unwrap();
let col = elts[subjects.1].column_index.unwrap();
gram_to_be.push_sym(row, col, *value);
}
};
if let Present { value, .. } = set_pt {
let subjects = reg.subjects;
glen marked this conversation as resolved Outdated
glen commented 2025-02-26 00:29:53 +00:00
Outdated
Review
Copy link

Please illuminate me: Why do we not need to write SpecifiedValue::Absent here? I mean, I like the brevity, but I think I am unclear about when the bare variants are in scope...

Please illuminate me: Why do we not need to write `SpecifiedValue::Absent` here? I mean, I like the brevity, but I think I am unclear about when the bare variants are in scope...
Vectornaut commented 2025-02-26 20:36:56 +00:00
Outdated
Review
Copy link

The declaration

use SpecifiedValue::*;

puts all variants in scope. I currently have it just after the definition of SpecifiedValue, because it felt out of context elsewhere. For example, I usually expect use declarations at the top of the file to be external.

The declaration ``` use SpecifiedValue::*; ``` puts all variants in scope. I currently have it just after the definition of `SpecifiedValue`, because it felt out of context elsewhere. For example, I usually expect use declarations at the top of the file to be external.
glen commented 2025-02-27 03:44:05 +00:00
Outdated
Review
Copy link

Ah, the fact that use is sort of "lost" in the midst of the file suggests that SpecifiedValue should be in its own source file and imported here, so the relevant use is up where one would generally look. assembly.rs is getting pretty huge anyway... What are your thought about that factoring?

Ah, the fact that `use` is sort of "lost" in the midst of the file suggests that SpecifiedValue should be in its own source file and imported here, so the relevant `use` is up where one would generally look. assembly.rs is getting pretty huge anyway... What are your thought about that factoring?
Vectornaut commented 2025-02-27 08:23:18 +00:00
Outdated
Review
Copy link

Specified values and assemblies do seem like pretty independent concepts, so putting SpecifiedValue in its own module might be reasonable.

Specified values and assemblies do seem like pretty independent concepts, so putting `SpecifiedValue` in its own module might be reasonable.
glen commented 2025-03-01 02:07:42 +00:00
Outdated
Review
Copy link

Are you doing so in this PR? That's my (relatively mild) recommendation.

Are you doing so in this PR? That's my (relatively mild) recommendation.
Vectornaut commented 2025-03-01 02:09:08 +00:00
Outdated
Review
Copy link

Yes, I'll do this—I was just waiting for a go-ahead from you. I should've mentioned that in the last comment.

Yes, I'll do this—I was just waiting for a go-ahead from you. I should've mentioned that in the last comment.
👍 1
Vectornaut commented 2025-03-03 21:19:19 +00:00
Outdated
Review
Copy link

Done (309b088).

Done (309b088).
let row = elts[subjects.0].column_index.unwrap();
let col = elts[subjects.1].column_index.unwrap();
gram_to_be.push_sym(row, col, *value);
}
});
}
});