Skip to content

Remove LOG_EQ to fix #10171#11386

Merged
trivialfis merged 2 commits into
dmlc:masterfrom
apmorton:patch-1
Apr 7, 2025
Merged

Remove LOG_EQ to fix #10171#11386
trivialfis merged 2 commits into
dmlc:masterfrom
apmorton:patch-1

Conversation

@apmorton

@apmorton apmorton commented Apr 4, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@trivialfis trivialfis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for looking into this. I think the check is useful for preventing silent error. If the error is not checked, then the logic of this function is broken as it relies on the returned error.

Could you please share the underlying cause of an error being leaked into XGBoost?

@apmorton

apmorton commented Apr 5, 2025

Copy link
Copy Markdown
Contributor Author

In some cases it seems cudaPointerGetAttributes is returning cudaSuccess and not setting the last error, which means if something else has previously caused an error with cuda, you blow up here.

I don't really know what the cause of that previous error is, but the situation is this:

  • on a machine using VMware horizon (which exposes a vgpu that is cuda enabled, but cuda not working)
  • using xgboost compiled for cuda, because when running on grid compute we want to use cuda
  • running some unit tests that attempt to use xgboost on CPU
  • attempt to construct DMatrix object with normal numpy array (in CPU memory)
  • blows up on this check, completely preventing use of xgboost

@apmorton

apmorton commented Apr 5, 2025

Copy link
Copy Markdown
Contributor Author

I suppose alternatively you could call cudaGetLastError before cudaPointerGetAttributes in addition to the existing check.

@trivialfis

Copy link
Copy Markdown
Member

I suppose alternatively you could call cudaGetLastError before cudaPointerGetAttributes in addition to the existing check.

This sounds like a better workaround, would you like to help make the change?

@apmorton

Copy link
Copy Markdown
Contributor Author

@hcho3 can this be cherry-picked into the next 3.x release please?

@hcho3 hcho3 mentioned this pull request May 30, 2025
10 tasks
@hcho3

hcho3 commented May 30, 2025

Copy link
Copy Markdown
Collaborator

I created #11494 to keep track of the upcoming patch release. Until the new patch release is out, consider using a nightly distribution instead: https://xgboost.readthedocs.io/en/release_3.0.0/install.html#id1

trivialfis pushed a commit to trivialfis/xgboost that referenced this pull request Jul 23, 2025
trivialfis pushed a commit to trivialfis/xgboost that referenced this pull request Jul 23, 2025
trivialfis added a commit that referenced this pull request Jul 23, 2025
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.

3 participants