Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

surfchem - modified ck,py, production.py, utilities.py and converter.py #486

Open
wants to merge 15 commits into
base: development
Choose a base branch
from

Conversation

jAnirudh
Copy link
Contributor

Reworked the CEPTR modules generating mechanism.cpp to work with heterogeneous reaction mechanisms.

  • Methods changed/added in:
    1. ck.py: ckncf, cksyme_str, cksyms_str, ckkfkr
    2. production.py: progress_rate_fr
    3. utilities.py: Added get_function_prefix, get_phase, def get_element_id, get_atomic_weight, get_element_names methods
  • Added a NUM_QSS_GAS_SPECIES preprocessor directive to converter.py along with changes corresponding to above additions/modifications
  • Regenerated all non-qss mechanisms

@marchdf
Copy link
Contributor

marchdf commented Apr 16, 2024

@jAnirudh can you rebase your branch on the latest? Once this looks good, I can update the qss chems as well.

@marchdf marchdf requested a review from nickwimer April 16, 2024 17:42
@baperry2
Copy link
Contributor

Just a thought: as this work will be implemented over a series of PRs, maybe it would be helpful to open an issue to track the progress and plans, similar to what we did here: #446

@jAnirudh
Copy link
Contributor Author

Just a thought: as this work will be implemented over a series of PRs, maybe it would be helpful to open an issue to track the progress and plans, similar to what we did here: #446

@baperry2 Sure, I'll put together a plan and open an issue.

Meanwhile, @marchdf could you please regenerate the qss mechanisms for me? The qss files are still somehow changing on my linux box.

@marchdf
Copy link
Contributor

marchdf commented Apr 29, 2024

@nickwimer and @jAnirudh Does this diff look right for the QSS mech? I am a little confused by the numbers. Actually maybe it's ok. Though I would want a second pair of eyes on this. I pushed the changes if you want to see them all.

Screenshot 2024-04-29 at 8 42 42 AM

@jAnirudh
Copy link
Contributor Author

@nickwimer and @jAnirudh Does this diff look right for the QSS mech? I am a little confused by the numbers. Actually maybe it's ok. Though I would want a second pair of eyes on this. I pushed the changes if you want to see them all.
Screenshot 2024-04-29 at 8 42 42 AM

I had introduced NUM_QSSA_GAS_SPECIES without realizing that the QSS species are a subset of NUM_GAS_SPECIES. Here I roll back the definition of NUM_QSSA_GAS_SPECIES to NUM_GAS_SPECIES and add NUM_QSS_SPECIES with the intent that hard coded calls, if any, coming from species_info.n_qssa_species can be replaced by NUM_QSS_SPECIES down the line.

@marchdf
Copy link
Contributor

marchdf commented Apr 29, 2024

@jAnirudh and @nickwimer I regenerated the QSS mechs with the latest. Feels like we need to confirm that all is well.

@jAnirudh
Copy link
Contributor Author

Hi @marchdf @baperry2 @nickwimer could you please share your comments on this PR for review/merge? Also tagging @ndeak here. Thanks.

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.

None yet

4 participants