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

Update to Chrono v9.0.0 #59

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

dariomangoni
Copy link

@dariomangoni dariomangoni commented May 22, 2024

Hi,
if you are interested these are the changes required to update HydroChrono to Chrono v9.0.0 (released few days ago) regarding both the code and the CMakeLists infrastructure as well.
There are also few other fixes about:

  • missing CHRONO_DATA_DIR definition
  • the sphere_decay_test that I think had a wrong offset in the test reference values
  • removed the number of iterations for those tests relying on direct solvers

All tests are running fine (tested only on Win though!)

@MShabara
Copy link

MShabara commented May 23, 2024

I was trying to run HydroChrono with Chrono v8.0, and I encountered these errors:
image Do you have any suggestions on how to fix this issue?

I saw your Pull request and cloned your project and used Chrono v9.0, however, I am getting this errors:
image

The file 'chrono/core/ChMathematics.h' is deleted in Chrono v9.0, this needs to be modified together any other changes in Chrono v9.0 files.

@dariomangoni
Copy link
Author

@MShabara: about your issue with Chrono v8 it clearly tells that you did not built Chrono with the required Irrlicht components. Did you?

About this pull request: the first error says that is looking for ChMathematics, but if you look at my code there is no call to such file. Same for Get_y.
Can you please tell me which files are giving such issue? You cut the screenshot.

@MShabara
Copy link

Hi @dariomangoni, I did build it with Chrono v.8 with the required Irrlicht components, I did it before in Sept. 2023 and it was working fine, I am not sure what changed.

About this pull request: I cloned your main repo by mistake, not the pr_chrono9. I tried your pr_chrono9 branch with both Chrono v.9 and Chrono main and I was able to build Hydrochrono with both, your pull request was done in a perfect timing.

@dav-og Could you please review this pull request?

@tridelat
Copy link
Collaborator

@dariomangoni Thanks for updating the code to Chrono 9.0.0!
The latest merged PR (and release before we switch to 9.0.0) created some small conflicts in wave_types.cpp with this branch. I rebased your branch on the lastest main locally and it compiles fine, let me know if you want me to push direclty onto your branch or if you could make those changes on your side.

@dariomangoni
Copy link
Author

@tridelat I took care of the rebase so there were no permission issues in (force) pushing on my fork.
Please let me know if this is fine. There were just two small conflicts that I resolved by picking your changes: I hope I didn't make any mistake.

@dariomangoni
Copy link
Author

Oh I just realized that since it is a pull request you could have pushed it with no permission issues!
If you feel more comfortable in pushing your changes (just to be sure) please do it without any hesitation!

@tridelat
Copy link
Collaborator

@dariomangoni it looks like it is now the same as my local rebase, so there is no need for me to push on your branch, all good!

@dariomangoni
Copy link
Author

@tridelat I pushed a small additional commit on the pr_chrono9 branch to address the missing export of CHRONO_DATA_DIR into the HydroChrono config file (it was set to PRIVATE while it should be PUBLIC)

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