fixes for sorption models and other#115
Merged
Merged
Conversation
merge unlimited strings
…the link error after the exception
…ase remake no resets the phase code
… and MaSDj[1] and MaSDj[2] with the charge of the species when clicking record calculate.
…modynamic properties
This was
linked to
issues
Apr 27, 2026
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR updates key/filter APIs to use std::string refs, adds JSON-driven allocations for dependent-component SD arrays, tightens numeric safeguards, adds runtime consistency warnings, restructures RCthermo error handling and linking, comments out two adsorption-copy loops, and introduces CI/Conda environment files and related CI changes. ChangesKey/filter API and callers
JSON-driven SD allocation
Phase sorption / MaSdj allocation and initialization
Runtime consistency checks
Reaction-DC computation control flow & error handling
Numerics and control tweaks
Disabled surface/adsorption copy blocks & minor cleanups
CI and environment
Sequence DiagramsequenceDiagram
participant RC as RCthermo
participant DB as Database
participant ODS as Ods/ObjectStorage
participant COMP as ComponentLoader
participant MEM as DynMemory
RC->>DB: ods_link / SetKey (non-current rc slot)
RC->>ODS: aDC->ods_link (link dependent records)
RC->>COMP: prepare water & recursive components
COMP-->>RC: components loaded
RC->>RC: perform thermodynamic dispatch
alt success
RC->>MEM: w_dyn_kill()
RC-->>DB: return results
else TError thrown
RC->>ODS: unlink structures
RC->>MEM: w_dyn_kill()
MEM-->>RC: throw Error(with pstate)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
GUI/Dataman/v_dbm2.cpp (1)
628-636:⚠️ Potential issue | 🟠 MajorAllocate
dSDval/rSDvalbefore deserializing them.
fromJsonValue()already ran by the time theseAlloc()calls execute. That leaves the import path correcting the shape only after reading the payload, so the freshly allocated buffer can replace the just-importedsdvaldata.🔧 Suggested reordering
QString obj_key(aObj[no]->GetKeywd()); - aObj[no]->fromJsonValue(dodAll[obj_key]); + if( obj_key == "dSDval") { + auto nSd = aObj[o_dcdim]->Get(0, 2); + aObj[o_dcsdval]->Alloc(nSd, 1, V_SD_VALEN ); + } + if( obj_key == "rSDval") { + auto nSd = aObj[o_redim]->Get(0, 7); + aObj[o_resdval]->Alloc(nSd, 1, V_SD_VALEN); + } + aObj[no]->fromJsonValue(dodAll[obj_key]); if( obj_key == "dSDref") { auto nSd = aObj[no]->GetN(); aObj[o_dcdim]->Put(nSd, 0, 2); } - if( obj_key == "dSDval") { - auto nSd = aObj[o_dcdim]->Get(0, 2); - aObj[o_dcsdval]->Alloc(nSd, 1, V_SD_VALEN ); - } if( obj_key == "rSDref") { auto nSd = aObj[no]->GetN(); aObj[o_redim]->Put(nSd, 0, 7); } - if( obj_key == "rSDval") { - auto nSd = aObj[o_redim]->Get(0, 7); - aObj[o_resdval]->Alloc(nSd, 1, V_SD_VALEN); - }Also applies to: 641-644
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GUI/Dataman/v_dbm2.cpp` around lines 628 - 636, The deserialization calls are occurring before the arrays are reallocated, so move the Alloc() calls to run before fromJsonValue() when obj_key is "dSDval" or "rSDval"; specifically, when handling obj_key == "dSDref" compute nSd via aObj[o_dcdim]->Get(0,2) (or from the existing dimension cell) and call aObj[o_dcsdval]->Alloc(nSd,1,V_SD_VALEN) (and the analogous Alloc for rSDval) prior to invoking aObj[no]->fromJsonValue(dodAll[obj_key]) so the incoming sdval data is deserialized into the correctly sized buffer. Ensure the same reordering is applied to the code block for lines 641-644.
🧹 Nitpick comments (4)
GUI/Services4/table_model.h (1)
147-150: Finish theconst std::string&migration for API consistency.Two filter entry points were not converted in this PR:
FixedKeyModelctor (Line 149) still takesconst char* akey_filter = "*"and forwards to aconst std::string¶meter — works via implicit conversion, but the inconsistency invites mistakes when adding new subclasses.KeysTableProxy::setFilter(Line 214; impl intable_model.cppL250) still takesconst char* new_filterand ultimately hands it toKeyModel::set_filter(const std::string&). All current callers inPhaseWizard.cpp(Lines 518, 539, 564) holdstd::string keyFilterand round-trip throughkeyFilter.c_str()only because of this signature.Aligning both signatures eliminates the unnecessary
.c_str()hops at call sites and the mixed style.♻️ Proposed alignment
- explicit FixedKeyModel( QObject * parent, size_t irt, - const TCStringArray& keys_to_select, - const char* akey_filter = "*" ): + explicit FixedKeyModel( QObject * parent, size_t irt, + const TCStringArray& keys_to_select, + const std::string& akey_filter = "*" ): KeyModel(parent, irt, akey_filter)- void setFilter( QWidget *parent, const char* new_filter ); + void setFilter( QWidget *parent, const std::string& new_filter );Then update
KeysTableProxy::setFilterintable_model.cppand the three call sites inPhaseWizard.cppto drop the.c_str().Also applies to: 214-214
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GUI/Services4/table_model.h` around lines 147 - 150, Change the two remaining filter entrypoints to take const std::string& instead of const char*: update the FixedKeyModel constructor signature (FixedKeyModel(..., const char* akey_filter = "*")) to accept const std::string& akey_filter = std::string("*") and forward that to KeyModel, and change KeysTableProxy::setFilter(const char* new_filter) to setFilter(const std::string& new_filter) so it can call KeyModel::set_filter(const std::string&) directly; then update all callers (PhaseWizard.cpp call sites that currently use keyFilter.c_str()) to pass the std::string keyFilter directly (remove .c_str()) and rebuild to ensure no implicit const char* conversions remain.GUI/Dialogs4/PhaseWizard.cpp (1)
134-139: Drop the commented-out pkey-based filter.The two
///-commented lines preserve the previous aggregate-state-prefixed filter logic. If the intentional behavior change is to widen the filter to all aggregate states, leaving the old expression as a comment just adds noise (and rots ifpkeyever changes shape). VCS history already preserves it.♻️ Proposed cleanup
//Page 2 - QString filter = QString("*:*:*:*:"); - ///if( pkey ) - /// filter = QChar(pkey[0]) + QString( pkey[1] != ':' ? "*" : "") + QString(":*:*:*:"); + QString filter = QStringLiteral("*:*:*:*:"); ui->checkRDFull->setChecked( rd_old_selection.empty() ); ui->lineRDFilter->setText(filter); rd_keys_model = new RDKeyModel(this, filter.toStdString());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GUI/Dialogs4/PhaseWizard.cpp` around lines 134 - 139, Remove the stale commented-out pkey-based filter lines and any related dead comment noise around the filter initialization in PhaseWizard::constructor (the QString filter declaration, ui->lineRDFilter->setText(filter) and rd_keys_model = new RDKeyModel(... ) are the active code paths); specifically delete the two lines starting with "///if( pkey )" and "/// filter = QChar(pkey[0]) ..." so the code only sets filter = QString("*:*:*:*:") and constructs RDKeyModel with that value, leaving ui->checkRDFull and ui->lineRDFilter unchanged.GUI/Dialogs4/ProcessWizard.cpp (1)
1810-1824: Clean up thefabs(static_cast<long int>(...))pattern; return type stillint.Two small concerns in
getNPoints:
Line 1824 mixes integer and floating-point absolute value: the
static_cast<long int>truncates the double result, thenfabs(which has no nativelong intoverload) converts it back todouble, and the assignment narrows tolong intagain. That's two needless conversions and a round-trip through floating point. Either computefabson the double expression and cast once, or usestd::labs/std::abson the integer. Preferred form:Line 1810 widens the local to
long intbut the function (and headerProcessWizard.h:70) still returnsint, soreturn nP;performs an implicit narrowing conversion. Values are bounded by9999(line 1830), so this is harmless in practice but may trip-Wconversion. If the intent was a wider type, the return type and header should be widened too; otherwise revertingnPtointis simpler.♻️ Proposed cleanup
- long int nP = 1; + int nP = 1; from = ui->tIters->item(0,col)->data(Qt::DisplayRole).toDouble(); until = ui->tIters->item(1,col)->data(Qt::DisplayRole).toDouble(); step = ui->tIters->item(2,col)->data(Qt::DisplayRole).toDouble(); if( fabs(step) < 1e-30 ) { if( (fabs(from) > 1e-30 || fabs(until) > 1e-30) && !( getType() == 'G' && col == 6 )) nP = 1; // changed by DK 25.02.10 else nP = -1; } else { if( fabs(until) > 1e-30 ) - nP = fabs(static_cast<long int>((until-from)/step+1.000001)); + nP = static_cast<int>(std::fabs((until-from)/step + 1.000001)); else nP = -1; // changed by DK 21.05.10 }If a wider type is actually required, change the signature in both
ProcessWizard.handProcessWizard.cppconsistently (and adjust the bounds check on line 1830).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@GUI/Dialogs4/ProcessWizard.cpp` around lines 1810 - 1824, The code in getNPoints uses fabs(static_cast<long int>(...)) causing unnecessary cast round-trips and nP is a long int while the function returns int; fix by (1) computing the absolute value in double then casting once: e.g. double cnt = fabs((until - from)/step + 1.000001); nP = static_cast<int>(cnt); (use std::fabs) to replace the fabs(static_cast<long int>(...)) expression in the branch that sets nP, and (2) change the local nP declaration from long int to int (or alternatively change the function signature and header to return long int if a wider type is intended) so the local type matches the function return type (refer to nP and getNPoints).Modules/m_dcomp.cpp (1)
1258-1260: Include the actual row count in the warning to aid diagnosis.The message reports the expected
dc[q].Nsdbut not the actualaObj[o_dcsdref]->GetN()— without both numbers the log entry doesn't tell the user how the record is broken.♻️ Proposed change
- if(aObj[ o_dcsdref ]->GetN() != dc[q].Nsd) { - gui_logger->warn("record {} illegal Nsd {}", str_key, dc[q].Nsd); - } + if(aObj[ o_dcsdref ]->GetN() != dc[q].Nsd) { + gui_logger->warn("record {} illegal Nsd: expected {}, got {}", + str_key, dc[q].Nsd, aObj[ o_dcsdref ]->GetN()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Modules/m_dcomp.cpp` around lines 1258 - 1260, The warning in the conditional comparing aObj[o_dcsdref]->GetN() to dc[q].Nsd should include the actual row count; update the gui_logger->warn call in the block that checks if(aObj[o_dcsdref]->GetN() != dc[q].Nsd) to log both the expected dc[q].Nsd and the actual aObj[o_dcsdref]->GetN() (alongside str_key) so the message shows both values for diagnosis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Modules/m_phase2.cpp`:
- Around line 782-785: The code currently sets Masdj0 from php->MSDT[0][0]
without guarding against a zero value, which causes PI_DEN to become 0 for MaSdj
entries; change the guard so Masdj0 stays 1 unless php->MSDT exists and
php->MSDT[0][0] is nonzero (e.g., replace the if(php->MSDT) check with a test
that also requires php->MSDT[0][0] != 0) so that Masdj0 remains 1 as the
intended fallback used when populating MaSdj[i][PI_DEN].
In `@Modules/m_proces.cpp`:
- Line 1373: The tolerance passed to GoldenSection can be zero when pep->pXii[0]
== 0 because the current floor uses 1e-16*fabs(pep->pXii[0]); change the
tolerance computation at the GoldenSection call so it always yields a positive
floor: compute a safe_tol = max(pep->pXii[2], absolute_min_tol, tiny_rel *
max(fabs(pep->pXii[0]), fabs(pep->pXii[1]-pep->pXii[0]))) and pass that instead
of std::max(pep->pXii[2], 1e-16*fabs(pep->pXii[0])); use an absolute_min_tol
like 1e-12 (or similar) and tiny_rel like 1e-16 to ensure nonzero positive
tolerance even when pXii[0]==0 before constructing GoldenSection.
In `@Modules/m_reacdc.cpp`:
- Around line 304-307: In check_input, the call to MessageToSave() currently
drops its boolean return so a user Cancel doesn't stop processing; update
check_input to capture MessageToSave()'s return value and if it returns false,
immediately return/abort (like TryRecInp does) before proceeding to the
dependent-record reads; locate the MessageToSave() invocation in check_input and
implement the same cancel/false-handling logic used in TryRecInp to prevent
further reads when the user cancels.
- Around line 794-799: The early return after calling Calc_rDCD (when
rc[q].rDC[...] == SRC_DCOMP or SRC_REACDC) skips the try-block cleanup and leaks
the ParDC allocated by w_dyn_new(); instead of returning directly from inside
the try, ensure you run the same cleanup path: either call
w_dyn_kill(rcp->ParDC) (and reset rcp->ParDC = NULL) just before returning, or
replace the return with a jump/flag that exits the try to the existing cleanup
code so w_dyn_kill() runs; update the branch around Calc_rDCD to invoke
w_dyn_kill() (or route to the try’s normal exit) before exiting to prevent the
ParDC leak.
---
Outside diff comments:
In `@GUI/Dataman/v_dbm2.cpp`:
- Around line 628-636: The deserialization calls are occurring before the arrays
are reallocated, so move the Alloc() calls to run before fromJsonValue() when
obj_key is "dSDval" or "rSDval"; specifically, when handling obj_key == "dSDref"
compute nSd via aObj[o_dcdim]->Get(0,2) (or from the existing dimension cell)
and call aObj[o_dcsdval]->Alloc(nSd,1,V_SD_VALEN) (and the analogous Alloc for
rSDval) prior to invoking aObj[no]->fromJsonValue(dodAll[obj_key]) so the
incoming sdval data is deserialized into the correctly sized buffer. Ensure the
same reordering is applied to the code block for lines 641-644.
---
Nitpick comments:
In `@GUI/Dialogs4/PhaseWizard.cpp`:
- Around line 134-139: Remove the stale commented-out pkey-based filter lines
and any related dead comment noise around the filter initialization in
PhaseWizard::constructor (the QString filter declaration,
ui->lineRDFilter->setText(filter) and rd_keys_model = new RDKeyModel(... ) are
the active code paths); specifically delete the two lines starting with "///if(
pkey )" and "/// filter = QChar(pkey[0]) ..." so the code only sets filter =
QString("*:*:*:*:") and constructs RDKeyModel with that value, leaving
ui->checkRDFull and ui->lineRDFilter unchanged.
In `@GUI/Dialogs4/ProcessWizard.cpp`:
- Around line 1810-1824: The code in getNPoints uses fabs(static_cast<long
int>(...)) causing unnecessary cast round-trips and nP is a long int while the
function returns int; fix by (1) computing the absolute value in double then
casting once: e.g. double cnt = fabs((until - from)/step + 1.000001); nP =
static_cast<int>(cnt); (use std::fabs) to replace the fabs(static_cast<long
int>(...)) expression in the branch that sets nP, and (2) change the local nP
declaration from long int to int (or alternatively change the function signature
and header to return long int if a wider type is intended) so the local type
matches the function return type (refer to nP and getNPoints).
In `@GUI/Services4/table_model.h`:
- Around line 147-150: Change the two remaining filter entrypoints to take const
std::string& instead of const char*: update the FixedKeyModel constructor
signature (FixedKeyModel(..., const char* akey_filter = "*")) to accept const
std::string& akey_filter = std::string("*") and forward that to KeyModel, and
change KeysTableProxy::setFilter(const char* new_filter) to setFilter(const
std::string& new_filter) so it can call KeyModel::set_filter(const std::string&)
directly; then update all callers (PhaseWizard.cpp call sites that currently use
keyFilter.c_str()) to pass the std::string keyFilter directly (remove .c_str())
and rebuild to ensure no implicit const char* conversions remain.
In `@Modules/m_dcomp.cpp`:
- Around line 1258-1260: The warning in the conditional comparing
aObj[o_dcsdref]->GetN() to dc[q].Nsd should include the actual row count; update
the gui_logger->warn call in the block that checks if(aObj[o_dcsdref]->GetN() !=
dc[q].Nsd) to log both the expected dc[q].Nsd and the actual
aObj[o_dcsdref]->GetN() (alongside str_key) so the message shows both values for
diagnosis.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f989534d-7194-4482-81cc-f2e7a2d59206
📒 Files selected for processing (13)
GUI/Dataman/v_dbm2.cppGUI/Dataman/v_module.cppGUI/Dialogs4/PhaseWizard.cppGUI/Dialogs4/ProcessWizard.cppGUI/Services4/table_model.cppGUI/Services4/table_model.hModules/Submods/ms_mtparm.cppModules/Submods/ms_system.cppModules/m_dcomp.cppModules/m_phase.cppModules/m_phase2.cppModules/m_proces.cppModules/m_reacdc.cpp
💤 Files with no reviewable changes (2)
- GUI/Dataman/v_module.cpp
- Modules/Submods/ms_mtparm.cpp
gdmiron
approved these changes
Apr 28, 2026
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Removed redundant allocation for dSDval and rSDval.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
…version pinning step
… add solver and path conflict settings
…tion and dependencies
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci-build.yml (2)
49-58: ⚡ Quick winConsider adding error handling to prevent silent failures.
The multi-line run command executes multiple critical setup steps, but without explicit error handling, a failure in an early command might not prevent later commands from running, potentially masking issues.
🛡️ Suggested change to add error handling
- name: Configuring Conda Environment shell: bash -l {0} run: | + set -e conda config --set always_yes yes conda config --set changeps1 no conda config --set solver libmamba conda config --set path_conflict warn conda clean -a -y conda-lock -f environment.yml -p ${{ matrix.conda_platform }} conda-lock install --name GEMSGUI conda-lock.yml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-build.yml around lines 49 - 58, The "Configuring Conda Environment" run block can silently continue on failures; modify the multi-line run script so failures abort immediately by enabling strict bash error handling (e.g., set -euo pipefail) and optionally add a trap to log/fail on errors before the conda commands (affecting the block that runs conda config, conda clean, conda-lock -f, and conda-lock install). Ensure the run’s shell setting still matches (shell: bash -l {0}) and place the strict-mode line at the top of that run block so any failing command in the conda setup stops the job and surfaces the error.
45-47: ⚡ Quick winPin the conda-lock version for reproducibility.
Installing
conda-lockwithout a version constraint may lead to build inconsistencies if conda-lock releases breaking changes or generates different lock files across versions.📌 Suggested change to pin the version
- name: Install conda-lock shell: bash -l {0} - run: pip install conda-lock + run: pip install conda-lock==2.5.7🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-build.yml around lines 45 - 47, The CI step named "Install conda-lock" currently runs "pip install conda-lock" unpinned; update that step to pin conda-lock to a specific, known-good version (for example use "conda-lock==2.1.0" or another verified release) so builds are reproducible and won't break on upstream changes; modify the run command in the "Install conda-lock" step to install the pinned version (or use a project-wide variable for the version if you prefer centralized control).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci-build.yml:
- Around line 40-43: Confirm that the GitHub Action reference
conda-incubator/setup-miniconda@v4 is a valid published version (replace with a
specific released tag if not) and, if it’s stable, keep or pin that exact tag;
also review and decide whether auto-activate: true should remain by testing how
setup-miniconda’s auto-activation interacts with the later explicit "conda
activate GEMSGUI" (remove the manual "conda activate GEMSGUI" if auto-activate
already activates the intended environment or disable auto-activate and keep the
explicit activation if you need deterministic behavior).
---
Nitpick comments:
In @.github/workflows/ci-build.yml:
- Around line 49-58: The "Configuring Conda Environment" run block can silently
continue on failures; modify the multi-line run script so failures abort
immediately by enabling strict bash error handling (e.g., set -euo pipefail) and
optionally add a trap to log/fail on errors before the conda commands (affecting
the block that runs conda config, conda clean, conda-lock -f, and conda-lock
install). Ensure the run’s shell setting still matches (shell: bash -l {0}) and
place the strict-mode line at the top of that run block so any failing command
in the conda setup stops the job and surfaces the error.
- Around line 45-47: The CI step named "Install conda-lock" currently runs "pip
install conda-lock" unpinned; update that step to pin conda-lock to a specific,
known-good version (for example use "conda-lock==2.1.0" or another verified
release) so builds are reproducible and won't break on upstream changes; modify
the run command in the "Install conda-lock" step to install the pinned version
(or use a project-wide variable for the version if you prefer centralized
control).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 02766095-29e9-4134-abde-a80b38986ff9
📒 Files selected for processing (3)
.github/workflows/ci-build.yml.gitignoreenvironment.yml
💤 Files with no reviewable changes (1)
- .gitignore
✅ Files skipped from review due to trivial changes (1)
- environment.yml
Comment on lines
+40
to
+43
| uses: conda-incubator/setup-miniconda@v4 | ||
| with: | ||
| auto-activate-base: false | ||
| channels: conda-forge, defaults | ||
| channel-priority: true | ||
| auto-activate: true | ||
| channels: conda-forge |
There was a problem hiding this comment.
Verify the setup-miniconda@v4 version exists and assess auto-activate redundancy.
The upgrade to setup-miniconda@v4 should be verified to ensure this version is stable and available. Additionally, auto-activate: true is configured here, but line 64 still includes conda activate GEMSGUI, which may be redundant.
Run the following to verify the action version exists:
Does conda-incubator/setup-miniconda@v4 exist on GitHub Actions marketplace?
Also verify the behavior of auto-activate with the following:
setup-miniconda auto-activate behavior and interaction with manual conda activate
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci-build.yml around lines 40 - 43, Confirm that the GitHub
Action reference conda-incubator/setup-miniconda@v4 is a valid published version
(replace with a specific released tag if not) and, if it’s stable, keep or pin
that exact tag; also review and decide whether auto-activate: true should remain
by testing how setup-miniconda’s auto-activation interacts with the later
explicit "conda activate GEMSGUI" (remove the manual "conda activate GEMSGUI" if
auto-activate already activates the intended environment or disable
auto-activate and keep the explicit activation if you need deterministic
behavior).
…iguration for macOS and Windows
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ReacDC - ask for saving changes before reading dependent records
GEMS project / database error message: Catch the error in RCthermo, detect the problem record name, and fix the link error after the exception
Added a warning message if an illegal reference size for DComp and ReactDC records
Reset sdval size from sdref size when json import
Surface data updated from the database, like other thermodynamic properties
Phase record: filling of the MaSDj[0] with the value given MsDT[0,0], and MaSDj[1] and MaSDj[2] with the charge of the species when clicking record calculate, and other improvements and fixes for sorption models
Prevent hanging in the golden section for the process: add the minimum possible value of the function tolerance
Summary by CodeRabbit
Bug Fixes
Chores