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

Use RavenHydroFramework from official GitHub source #24

Merged
merged 40 commits into from
Mar 5, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Jan 11, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)

What kind of change does this PR introduce?

  • Points the RavenHydroFramework source files to the official GitHub project
  • Updates the version of Raven to 3.8
  • Updates the CMake configuration to use git-based clones
  • Both the version number and the description are now dynamic (using hatch-fancy-pypi-readme and setuptools_scm).
  • Support has been extended to Python3.12
  • Now uses scikit-build-core>=0.8.0

Does this PR introduce a breaking change?

Yes. The copied source files have been removed. There is more integration with the actual project source files.

@Zeitsperre Zeitsperre added the enhancement New feature or request label Jan 11, 2024
@Zeitsperre Zeitsperre self-assigned this Jan 11, 2024
@Zeitsperre
Copy link
Collaborator Author

Zeitsperre commented Mar 1, 2024

@huard

I've spent way too much time on this today. Here's what I've figured out:

  • Running the cmake commands directly on the upstream codebase RavenHydroFramework works fine (so long as the STANDALONE and COMPILE_EXE flags are there).
  • When compiling things here, it's clear that the functions defined in RavenMain.cpp are not being passed to other files. ExitGracefully() is only found in a .cpp file and not in a header, and this file isn't included in a bunch of .cpp files that make references to that function.
  • Unsurprisingly, if you copy and paste the following into the top-matter of all .cpp files, the ExitGracefully() error disappears:
  #ifdef STANDALONE
      #include "GracefulEndStandalone.h"
  #elif BMI_LIBRARY
      #include "GracefulEndBMI.h"
  #endif 

I've been trying to read up on ways of passing the objects from RavenMain.cpp to all other files, but I have no idea if that's even what we want. I get the impression that the problem has to do with scopes (perhaps vanilla cmake is a lot less strict about ensuring that files are included explicitly?).

@analytophile am I on the right track here?

@huard
Copy link
Contributor

huard commented Mar 4, 2024

@Zeitsperre

I suspect this is a false lead. Your installation setup works on master, and the code compiles fine using the original cmakelist. I believe there is a quirk related to the source files location introduced in the git fetch mechanism that we need to identify. I'll take a look later this week.

Copy link
Contributor

@huard huard left a comment

Choose a reason for hiding this comment

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

I suggest to use the same target names and env variables as in the original cmakelist to reduce confusion.

Targets:

  • libraven
  • ravenbmi
  • Raven
option(COMPILE_LIB "If ON, will create a dynamic lib file (default: OFF)" OFF)
option(COMPILE_EXE "If ON, will create a executable file (default: ON)" ON)
option(PYTHON, "If ON, will create a share library for python (default: OFF)" OFF)

@Zeitsperre Zeitsperre marked this pull request as ready for review March 5, 2024 18:12
@Zeitsperre
Copy link
Collaborator Author

Zeitsperre commented Mar 5, 2024

Seems to be working! The only issue I can see is that the model reports a non-fatal error:

 3.8.0 w/ netCDF
============== Early Gracefully Exit ==============
Error Statement: Version check
Error Code: 7
---------------------------------------------------

@huard any idea what that could be?

EDIT: It's nothing. Looks like it's simply the exit code for SIMULATION_DONE:

enum exitcode
{
  BAD_DATA,       ///< For bad input provided by user (requires immediate exit from program)
  BAD_DATA_WARN,  ///< For bad input provided by user (requires shutdown prior to simulation)
  RUNTIME_ERR,    ///< For runtime error (bad programming)
  FILE_OPEN_ERR,  ///< For bad file open (requires immediate exit)
  RAVEN_OPEN_ERR, ///< for bad RavenErrors.txt file open
  STUB,           ///< Stub function
  OUT_OF_MEMORY,  ///< When out of memory
  SIMULATION_DONE ///< Upon completion of the simulation
};

@Zeitsperre
Copy link
Collaborator Author

Needed to set the fetch-depth for setuptools_scm. See: pypa/setuptools-scm#414

@Zeitsperre Zeitsperre merged commit ff59414 into main Mar 5, 2024
19 checks passed
@Zeitsperre Zeitsperre deleted the from-official-git branch March 5, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Raven 3.8
2 participants