Skip to content

docs: Properties chapter of the Ixa Book#866

Merged
RobertJacobsonCDC merged 2 commits into
mainfrom
RobertJacobsonCDC_853_properties_docs
May 14, 2026
Merged

docs: Properties chapter of the Ixa Book#866
RobertJacobsonCDC merged 2 commits into
mainfrom
RobertJacobsonCDC_853_properties_docs

Conversation

@RobertJacobsonCDC
Copy link
Copy Markdown
Collaborator

@RobertJacobsonCDC RobertJacobsonCDC commented Apr 30, 2026

This PR adds a chapter all about properties to the Ixa Book, including a section on implementing Eq and Hash. The source file is docs/book/src/topics/properties.md. (The GitHub rendered markdown file is here.)

To render the book locally:

mise run docs

Then open website/book/topics/properties.html.

@RobertJacobsonCDC RobertJacobsonCDC force-pushed the RobertJacobsonCDC_853_properties_docs branch from cc85c13 to ed53d56 Compare April 30, 2026 04:12
github-actions Bot added a commit that referenced this pull request Apr 30, 2026
github-actions Bot added a commit that referenced this pull request Apr 30, 2026
@CDCgov CDCgov deleted a comment from github-actions Bot Apr 30, 2026
@CDCgov CDCgov deleted a comment from github-actions Bot Apr 30, 2026
@RobertJacobsonCDC RobertJacobsonCDC force-pushed the RobertJacobsonCDC_853_properties_docs branch from ed53d56 to e8d152b Compare April 30, 2026 15:10
github-actions Bot added a commit that referenced this pull request Apr 30, 2026
@CDCgov CDCgov deleted a comment from github-actions Bot Apr 30, 2026
@RobertJacobsonCDC RobertJacobsonCDC linked an issue May 1, 2026 that may be closed by this pull request
github-actions Bot added a commit that referenced this pull request May 1, 2026
@CDCgov CDCgov deleted a comment from github-actions Bot May 2, 2026
Copy link
Copy Markdown
Contributor

@swo swo left a comment

Choose a reason for hiding this comment

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

Overall I think this is a very helpful chapter. I learned a lot while reading it!

As I read, I fiddled with the text; see my suggestions. It's mostly minor reorderings or rewordings, in ways that made things easier for me to understand.

A few things I wanted to call out:

  • All properties take on values immediately upon creation of an entity. Is it idiomatic ixa to make properties that are Options for things you haven't implemented yet? Or is that straying into data plugin territory?
  • You set up the explicit vs. constant vs. derived divide, but I think it should really just be derived vs. not.
    • I think it's very intuitive that a property can have a default value upon instantiation. This just feels like a nice convenience, not something deep about classes of properties.
    • If you do keep the three-way division, I'd recommend against calling them "constant," which is confusing, because you can set them to whatever.
    • While I'm here I'll also recommend replacing "non-derived" with "base" or "source," or something that clarifies what it actually is.
  • I would re-organize a little bit, to start with non-derived properties and define_property! and how to use them, then go one step deeper and say that define_property! is just a nice wrapper around impl_property!, and then go into derived properties only after that. I can take a go at that if you think it would be helpful.
  • Does Beau's thing, about wanting to use an enum for edge types, to avoid duplicating a value associated with that edge, match with the motivation for canonical values?
  • I think it's useful to know that the same property type (eg Age) can be defined for multiple entities, and it's nice to know that that happens via implementing eg Property<Person>, but it's a little confusing because no one should be every writing impl Property<Person> for Age themsevles.

@RobertJacobsonCDC RobertJacobsonCDC force-pushed the RobertJacobsonCDC_853_properties_docs branch from 92b9a5b to 8bb31e9 Compare May 8, 2026 17:37
@github-actions

This comment was marked as outdated.

github-actions Bot added a commit that referenced this pull request May 8, 2026
@swo
Copy link
Copy Markdown
Contributor

swo commented May 12, 2026

@RobertJacobsonCDC I recommend you merge this; I was pointing an analyst to it, and I think it's way better than nothing!

Copy link
Copy Markdown
Collaborator

@k88hudson-cfa k88hudson-cfa left a comment

Choose a reason for hiding this comment

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

Looks good with follow-up comments from scott

@RobertJacobsonCDC
Copy link
Copy Markdown
Collaborator Author

Thanks for all this feedback, @swo !

Some comments:

Is it idiomatic ixa to make properties that are Options for things you haven't implemented yet?

Yes, and this is worth adding to this chapter. I'll add a section to make this explicit. (It's mentioned in the section on custom display functions.)

You set up the explicit vs. constant vs. derived divide, but I think it should really just be derived vs. not.

Well, not exactly. I make this distinction for initialization modes only. But if you find it confusing, others will to. Let me tweak some language to make this more clear.

I'd recommend against calling them "constant," which is confusing

This language comes from our need to distinguish between a "default" that is dynamically computed versus a default that is provided as a static constant in the property definition. So the adjective refers to the value being assigned to the property rather than the property itself.

We no longer support dynamically computing default values, so the distinction is vestigial. It probably makes sense to just call it "default value". But that's an API change out of scope of this PR. I'll open a separate ticket for it.

I would re-organize a little bit...

I think I did a bit of reorganizing since your review that makes it closer to your suggestion. I'm doing another pass. Let me push my changes incorporating your feedback.

But actually, if you're feeling motivated to work on Ixa documentation, we probably have a greater need for user guide material for topics that aren't covered at all. I made a docs tag for issues related to documentation. It'd be really useful to know which topics you and your team run into that don't yet have any user-guide coverage but that most new users will want to learn about. We've sort of adopted a just-in-time documentation strategy of waiting until we identify a need before spending the effort, just as a pragmatic way of prioritizing work.

Of course, any and all feedback and contributions are gold.

Does Beau's thing, about wanting to use an enum for edge types, to avoid duplicating a value associated with that edge, match with the motivation for canonical values?

I don't know. I'm not familiar with Beau's design for edge types.

but it's a little confusing because no one should be every writing impl Property<Person> for Age themsevles.

Right. I try to put this front and center when I introduce the list of macros. I also use the example of using the same type as a property for multiple entity types under the "When to use impl_property!" section.

For some of these things, I wonder if it would be helpful to have a whole chapter devoted to answering questions of the form, "How do I do X?" Or maybe a chapter of common "modeling patterns"? (Or both?)

@RobertJacobsonCDC RobertJacobsonCDC force-pushed the RobertJacobsonCDC_853_properties_docs branch from 8bb31e9 to 7a2143f Compare May 14, 2026 19:54
@github-actions
Copy link
Copy Markdown

Benchmark Results

Hyperfine

Command Mean [ms] Min [ms] Max [ms] Relative
large_sir::baseline 3.0 ± 0.1 2.9 3.5 1.00
large_sir::entities 6.6 ± 0.1 6.4 7.1 2.23 ± 0.07

Criterion

Regressions (slower)
Group Bench Param Change CI Lower CI Upper
sample_entity sample_entity_single_property_indexed 10000 71.189% 70.184% 72.200%
sample_entity sample_entity_single_property_indexed 100000 65.512% 64.668% 66.344%
sampling sampling_single_known_length_entities 53.724% 52.943% 54.469%
sampling count_and_sampling_single_known_length_entities 45.445% 44.525% 46.283%
sample_entity sample_entity_multi_property_indexed 10000 33.423% 32.528% 34.319%
sample_entity sample_entity_multi_property_indexed 100000 30.589% 29.605% 31.607%
sample_entity sample_entity_multi_property_indexed 1000 29.656% 27.875% 31.326%
sample_entity sample_entity_single_property_indexed 1000 22.567% 18.701% 26.634%
indexing query_people_single_indexed_property_entities 9.128% 8.989% 9.261%
algorithm_benches algorithm_sampling_single_known_length 5.982% 5.536% 6.314%
indexing with_query_results_single_indexed_property_entities 4.690% 4.120% 5.275%
sample_entity sample_entity_single_property_unindexed 10000 2.939% 2.578% 3.241%
algorithm_benches algorithm_sampling_multiple_known_length 2.694% 2.375% 3.067%
sampling count_and_sampling_single_unindexed_concrete_plus_derived_entiti 2.507% 2.276% 2.713%
indexing query_people_multiple_individually_indexed_properties_entities 2.210% 2.032% 2.382%
examples example-basic-infection 1.657% 1.188% 2.082%
indexing with_query_results_indexed_multi-property_entities 1.425% 1.179% 1.669%
indexing query_people_count_multiple_individually_indexed_properties_enti 1.369% 1.167% 1.554%
sampling sampling_single_unindexed_entities 1.262% 1.071% 1.517%
Improvements (faster)
Group Bench Param Change CI Lower CI Upper
algorithm_benches algorithm_sampling_single_l_reservoir -21.573% -21.669% -21.470%
large_dataset bench_filter_unindexed_entity -17.673% -21.318% -13.918%
large_dataset bench_match_entity -15.889% -16.056% -15.731%
indexing query_people_count_indexed_multi-property_entities -15.064% -15.937% -14.143%
counts concrete_plus_derived_unindexed_entities -12.498% -14.423% -10.511%
large_dataset bench_query_population_indexed_property_entities -7.979% -8.229% -7.499%
large_dataset bench_query_population_derived_property_entities -7.506% -8.068% -6.987%
indexing query_people_count_single_indexed_property_entities -7.476% -7.788% -7.125%
algorithm_benches algorithm_sampling_multiple_l_reservoir -7.085% -7.313% -6.817%
sample_entity sample_entity_whole_population 1000 -5.177% -5.483% -4.855%
large_dataset bench_query_population_multi_indexed_entities -2.493% -2.920% -2.101%
sampling sampling_single_l_reservoir_entities -2.434% -2.599% -2.261%
sample_entity sample_entity_whole_population 10000 -2.346% -2.745% -1.934%
counts single_property_unindexed_entities -2.175% -3.558% -1.077%
sample_entity sample_entity_whole_population 100000 -2.122% -2.559% -1.689%
counts index_after_adding_entities -2.036% -2.198% -1.866%
examples example-births-deaths -1.989% -2.268% -1.723%
sampling sampling_single_unindexed_concrete_plus_derived_entities -1.924% -2.237% -1.626%
sampling sampling_multiple_known_length_entities -1.786% -2.153% -1.279%
counts reindex_after_adding_more_entities -1.495% -1.730% -1.242%
Unchanged / inconclusive (CI crosses 0%)
Group Bench Param Change CI Lower CI Upper
large_dataset bench_filter_indexed_entity -9.094% -18.635% 0.557%
counts multi_property_unindexed_entities 2.264% 0.939% 3.691%
large_dataset bench_query_population_property_entities 2.085% 0.735% 3.649%
sample_entity sample_entity_single_property_unindexed 100000 -1.978% -3.562% -0.737%
counts single_property_indexed_entities -1.234% -2.058% -0.338%
algorithm_benches algorithm_sampling_single_rand_reservoir 0.942% 0.491% 1.376%
counts multi_property_indexed_entities -0.649% -1.016% -0.218%
indexing query_people_indexed_multi-property_entities 0.600% -0.117% 1.457%
indexing with_query_results_multiple_individually_indexed_properties_enti 0.542% 0.203% 0.846%
sample_entity sample_entity_single_property_unindexed 1000 0.374% -0.313% 1.189%
large_dataset bench_query_population_multi_unindexed_entities 0.368% -0.431% 0.907%
sampling sampling_multiple_unindexed_entities 0.320% 0.169% 0.414%
sampling sampling_multiple_l_reservoir_entities -0.189% -0.661% 0.459%

github-actions Bot added a commit that referenced this pull request May 14, 2026
@RobertJacobsonCDC RobertJacobsonCDC merged commit 1e0da1a into main May 14, 2026
22 checks passed
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.

Ixa Book chapter on properties

4 participants