Skip to content

Conversation

@YellowRoseCx
Copy link

@YellowRoseCx YellowRoseCx commented Dec 19, 2023

When importing a .kcpps file with the backend Use hipBLAS chosen, it would not select Use hipBLAS. This change corrects that.

If enabled, the argument --checkforupdates will fetch the KoboldCpp release page(via Github API) one time on start up via HTTPS and compare the latest version number with the current version number and notify the user if a new version is available.
A GUI button is shown on the Network tab. Disabled by default.

Fixed a mistake preventing hipBLAS from being autopicked on startup

When importing a .kcpps file with the backend Use hipBLAS chosen, it would not select Use hipBLAS. This change corrects that
If enabled, the argument --checkforupdates will fetch the KoboldCpp release page one time on start up via HTTPS and compare the latest version number with the current version number and notify the user if a new version is available.
A GUI button is shown on the Network tab.
@YellowRoseCx YellowRoseCx marked this pull request as draft December 19, 2023 05:11
@YellowRoseCx YellowRoseCx changed the title Add --checkforupdates argument & Fix bug when importing .kcpps saves with hipBLAS Add --checkforupdates argument, hipBLAS autopicking and .kcpps bug fixes Dec 19, 2023
@YellowRoseCx YellowRoseCx marked this pull request as ready for review December 19, 2023 06:40
@YellowRoseCx YellowRoseCx changed the title Add --checkforupdates argument, hipBLAS autopicking and .kcpps bug fixes Add --checkforupdates argument, hipBLAS autopicking and hipBLAS .kcpps bug fixes Dec 19, 2023
since sometimes/most of the time AMD GPU info is fetched with OpenCL, requiring ``CUDevicesNames[0]!=""`` prevents hipBLAS from being selected. Only check ``CUDevicesNames[0]!=""`` if Use CuBLAS in runopts
@LostRuins
Copy link
Owner

can I modify this to grab the fix PR for the HIPBlas bug fixes first?
or split it into 2 PRs?

I'm not so sure about the update checking stuff cause:

  1. it adds 2 more non-default python module dependencies.
  2. it requires a sync fetch, slowing down the execution/launch and possibly causing issues if they are not actually connected to the internet or on a slow network.
  3. It seems kind of unnecessary. There's already an update button in the GUI. It simply directs them to the repo to download the a new version.

@LostRuins LostRuins force-pushed the concedo_experimental branch from 1fa014b to c05d195 Compare December 21, 2023 13:25
LostRuins added a commit that referenced this pull request Dec 22, 2023
@LostRuins
Copy link
Owner

Cherry picked the HIPBlas fixes in 852ca78

@LostRuins LostRuins changed the base branch from concedo_experimental to concedo December 22, 2023 13:32
@LostRuins LostRuins changed the base branch from concedo to concedo_experimental December 22, 2023 13:32
@YellowRoseCx
Copy link
Author

YellowRoseCx commented Dec 23, 2023

can I modify this to grab the fix PR for the HIPBlas bug fixes first? or split it into 2 PRs?

I'm not so sure about the update checking stuff cause:

  1. it adds 2 more non-default python module dependencies.
  2. it requires a sync fetch, slowing down the execution/launch and possibly causing issues if they are not actually connected to the internet or on a slow network.
  3. It seems kind of unnecessary. There's already an update button in the GUI. It simply directs them to the repo to download the a new version.

The reason I made the check for updates argument was because the update button doesn't change color or notify when an update is available. I thought it only needed urllib3 added? Whats the second dependency?
I dont mind the cherry picking, but I think the way it was done didnt count as a commit from my account in terms of contribution count

@LostRuins
Copy link
Owner

what's the second dependency

urllib is fine as it's part of the default actually, the extra ones are colorama and requests.

I think the way it was done didnt count as a commit from my account in terms of contribution count

ah probably because I added and merged the specific changes as a patch instead of a whole commit. If you'd like, you can resubmit the changes as a separate PR and I'll merge that so you get the contrib.

@LostRuins LostRuins force-pushed the concedo_experimental branch 2 times, most recently from 0617bd8 to e2e8da0 Compare January 24, 2024 15:18
@YellowRoseCx YellowRoseCx deleted the concedo_exp-PR-updatecheck-fixkcpps branch April 10, 2024 15:17
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.

2 participants