Skip to content

Conversation

@mabruzzo
Copy link
Collaborator

To be reviewed after PR #463 is merged


This PR transcribes calc_grain_size_increment_1d. I couldn't help myself -- I cleaned up a bunch of logic (folding lots of logic that was explicitly repeated for every kind of injection pathway or for every kind of dust species into for-loops).

As of this PR, there is no logic outside of the inject_model subdirectory that directly references injection pathways by name. In other words, we are pretty close to being able to let users load an arbitrary set of injection pathways.

There is some unforunate consequences, but if we don't do this you end
up with some pretty confusing looking code... (the confusion primarily
arises if new_FrozenKeyIdxBiMap or drop_FrozenKeyIdxBiMap has different
behavior from other functions with similar names). I really think it
would be better to make this into a simple C++ class with a constructor
and destructor, but thats a topic for another time
…InfoEntry

This information will only be used after we transcribe
`calc_grain_size_increment_1d` (at which point we will remove the
corresponding constants from phys_constants.h and can entirely delete
the dust_const.def file
I moved them from initialize_dust_yields.cpp where the comments were out
of place to dust/grain_species_info.cpp
There's still a lot of work to be done (the type is incomplete and isn't
filled by anything at all)
@mabruzzo mabruzzo added the refactor internal reorganization or code simplification with no behavior changes label Dec 11, 2025
@mabruzzo mabruzzo changed the title [newchem-cpp] calc_grain_size_increment_1d [newchem-cpp] transcribe calc_grain_size_increment_1d Dec 11, 2025
@mabruzzo mabruzzo changed the base branch from main to newchem-cpp December 11, 2025 14:28
I was expecting this to cause gold-standard drift (but that doesn't seem
to be the case)
@mabruzzo mabruzzo force-pushed the ncc/transcribe_calc_grain_size_increment_1d branch from 554e23a to 1671ee3 Compare December 11, 2025 16:56
@mabruzzo
Copy link
Collaborator Author

ASIDE: I noticed that the dust-grain freefall test all systematically take slightly less time (think 0.2-0.4 seconds) than on the current HEAD of newchem-cpp. It's hard to definitively identify the source of speedup, but I think its because I cut a lot of unnecessary allocations out of this function

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

Labels

refactor internal reorganization or code simplification with no behavior changes

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant