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

Splitting forcing_code/ into extern/forcing_code/ and extern/pet/ #71

Closed
wants to merge 1 commit into from

Conversation

WanruWu-NOAA
Copy link
Contributor

This PR is for replacing the subdirectory forcing_code/ with adding extern/forcing_code/ from the aorc_bmi repo and adding extern/pet/ from the evapotranspiration repo directly so that all the code under the original forcing_code/ can be updated automatically in this new structure.

List to be committed:

new file:   .gitmodules
new directory:   extern/forcing_code/
new directory:   extern/pet/
deleted:    forcing_code/include/PEtAerodynamicMethod.h
deleted:    forcing_code/include/PEtCombinationMethod.h
deleted:    forcing_code/include/PEtEnergyBalanceMethod.h
deleted:    forcing_code/include/PEtPenmanMonteithMethod.h
deleted:    forcing_code/include/PEtPriestleyTaylorMethod.h
deleted:    forcing_code/include/aorc.h
deleted:    forcing_code/include/aorc_tools.h
deleted:    forcing_code/include/bmi_aorc.h
deleted:    forcing_code/include/bmi_pet.h
deleted:    forcing_code/include/pet.h
deleted:    forcing_code/include/pet_tools.h
deleted:    forcing_code/src/aorc.c
deleted:    forcing_code/src/bmi_aorc.c
deleted:    forcing_code/src/bmi_pet.c
deleted:    forcing_code/src/pet.c
modified:   CMakeLists.txt
modified:   README.md
modified:   src/main_cfe_aorc_pet.c
modified:   src/main_cfe_aorc_pet_rz_aet.cxx
modified:   src/main_pass_forcings.c

Additions

Adding two submodules extern/forcing_code and extern/pet:

Removals

Deleting forcing_code/:

  • forcing_code/include/PEtAerodynamicMethod.h
  • forcing_code/include/PEtCombinationMethod.h
  • forcing_code/include/PEtEnergyBalanceMethod.h
  • forcing_code/include/PEtPenmanMonteithMethod.h
  • forcing_code/include/PEtPriestleyTaylorMethod.h
  • forcing_code/include/aorc.h
  • forcing_code/include/aorc_tools.h
  • forcing_code/include/bmi_aorc.h
  • forcing_code/include/bmi_pet.h
  • forcing_code/include/pet.h
  • forcing_code/include/pet_tools.h
  • forcing_code/src/aorc.c
  • forcing_code/src/bmi_aorc.c
  • forcing_code/src/bmi_pet.c
  • forcing_code/src/pet.c

Changes

Modifying corresponding paths:

  • CMakeLists.txt
  • README.md
  • src/main_cfe_aorc_pet.c
  • src/main_cfe_aorc_pet_rz_aet.cxx
  • src/main_pass_forcings.c

Testing

  • tested in the ngen framework
  • tested on Linux

Notes

  • There have been some changes for cfe repository since the previous PR submitted for adding extern/forcing_code, which can be deleted/closed without further processing.
  • This new PR has combined both adding extern/forcing_code/ and adding extern/pet/, as well as deleting the original forcing_code/.

Todos

  • Can do further test if needed.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

	new file:   .gitmodules
	modified:   CMakeLists.txt
	modified:   README.md
	new file:   extern/forcing_code
	new file:   extern/pet
	deleted:    forcing_code/include/PEtAerodynamicMethod.h
	deleted:    forcing_code/include/PEtCombinationMethod.h
	deleted:    forcing_code/include/PEtEnergyBalanceMethod.h
	deleted:    forcing_code/include/PEtPenmanMonteithMethod.h
	deleted:    forcing_code/include/PEtPriestleyTaylorMethod.h
	deleted:    forcing_code/include/aorc.h
	deleted:    forcing_code/include/aorc_tools.h
	deleted:    forcing_code/include/bmi_aorc.h
	deleted:    forcing_code/include/bmi_pet.h
	deleted:    forcing_code/include/pet.h
	deleted:    forcing_code/include/pet_tools.h
	deleted:    forcing_code/src/aorc.c
	deleted:    forcing_code/src/bmi_aorc.c
	deleted:    forcing_code/src/bmi_pet.c
	deleted:    forcing_code/src/pet.c
	modified:   src/main_cfe_aorc_pet.c
	modified:   src/main_cfe_aorc_pet_rz_aet.cxx
	modified:   src/main_pass_forcings.c
@SnowHydrology
Copy link
Contributor

@WanruWu-NOAA does a clean build of this code run for you as-is? I'm getting a seg fault when trying to run:

./run_cfe.sh FORCINGPET
CFE running with option FORCINGPET
config file: ./configs/cat_87_bmi_config_cfe_pass.txt ./configs/cat_87_bmi_config_aorc.txt ./configs/cat_87_bmi_config_pet_pass.txt
allocating memory to store entire BMI structure for CFE
allocating memory to store entire BMI structure for AORC
allocating memory to store entire BMI structure for PET
Registering BMI CFE model
Registering BMI AORC model
Registering BMI PET model
Initializeing BMI CFE model
Initializeing BMI AORC model
AORC config file ./configs/cat_87_bmi_config_aorc.txt
./run_cfe.sh: line 38: 90499 Segmentation fault: 11  ./build/${exe_name} $args

@WanruWu-NOAA
Copy link
Contributor Author

WanruWu-NOAA commented Oct 27, 2022

@SnowHydrology Hmm, I couldn't run it, see the screenshot below,
image
then I tried this below:

[wanru.wu@nwcal-apd-dev1 cfe]$ which bash
/usr/bin/bash
[wanru.wu@nwcal-apd-dev1 cfe]$ vi run_cfe.sh
[wanru.wu@nwcal-apd-dev1 cfe]$ ./run_cfe.sh FORCINGPET
bash: ./run_cfe.sh: /usr/bin/bash^M: bad interpreter: No such file or directory

Ok, I googled to fix the "bad interpreter", here is what I got:

[wanru.wu@nwcal-apd-dev1 cfe]$ sed -i -e 's/\r$//' run_cfe.sh
[wanru.wu@nwcal-apd-dev1 cfe]$ ./run_cfe.sh FORCINGPET
CFE running with option FORCINGPET
config file: ./configs/cat_87_bmi_config_cfe_pass.txt ./configs/cat_87_bmi_config_aorc.txt ./configs/cat_87_bmi_config_pet_pass.txt
./run_cfe.sh: line 38: ./build/cfe_forcingpet: No such file or directory

@SnowHydrology
Copy link
Contributor

@WanruWu-NOAA, it doesn't look like the executable has been built. Did you follow the instructions to build FORCINGPET in the README?

@WanruWu-NOAA
Copy link
Contributor Author

WanruWu-NOAA commented Oct 27, 2022

@SnowHydrology thanks for the reminder :-) I followed this:

  git clone https://github.com/NOAA-OWP/cfe.git
  cd cfe
  git checkout ajk/sft_aet_giuh_merge
  mkdir build && cd build
  cmake ../ -DFORCINGPET=ON
  make
  sed -i -e 's/\r$//' run_cfe.sh
  ./run_cfe.sh FORCINGPET

and got this:

[wanru.wu@nwcal-apd-dev1 cfe]$ ./run_cfe.sh FORCINGPET
CFE running with option FORCINGPET
config file: ./configs/cat_87_bmi_config_cfe_pass.txt ./configs/cat_87_bmi_config_aorc.txt ./configs/cat_87_bmi_config_pet_pass.txt
allocating memory to store entire BMI structure for CFE
allocating memory to store entire BMI structure for AORC
allocating memory to store entire BMI structure for PET
Registering BMI CFE model
Registering BMI AORC model
Registering BMI PET model
Initializeing BMI CFE model
' could not be opened for reading
Initializeing BMI AORC model
AORC config file ./configs/cat_87_bmi_config_aorc.txt
File does not exist.
 Failed in function read_file_line_counts
' could not be opened for readinggs/cat87_01Dec2015.csv
Initializeing BMI PET model
File does not exist.
 Failed in function read_file_line_counts_pet
' could not be opened for reading
Get the information from the configuration here in Main
forcing file for the CFE module BMI
forcing file for the PET module BMI
forcing file for the AORC module ./forcings/cat87_01Dec2015.csv
The model time step size is: 3600.000000
looping through and calling update
#    ,            hourly ,  direct,   giuh ,lateral,  base,   total, storage, ice fraction, ice fraction
Time [h],rainfall [mm],runoff [mm],runoff [mm],flow [mm],flow [mm],discharge [mm],storage [mm],schaake [mm],xinan [-]
./run_cfe.sh: line 38: 35458 Segmentation fault      (core dumped) ./build/${exe_name} $args

@SnowHydrology
Copy link
Contributor

@WanruWu-NOAA, it looks like there's a bit of work left before I can review the PR

Copy link
Contributor

@SnowHydrology SnowHydrology left a comment

Choose a reason for hiding this comment

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

Does not run

@WanruWu-NOAA
Copy link
Contributor Author

WanruWu-NOAA commented Oct 28, 2022

@SnowHydrology Yeah, the test of compiling and running cfe which I did yesterday was for the current cfe version before any changes from this PR. I need to get the current cfe ./run_cfe.sh FORCINGPET work first then we can review this PR again.

@WanruWu-NOAA
Copy link
Contributor Author

@SnowHydrology I'm going to close this PR to avoid any confusion as it was basically same as the combination of PR#78 and PR#79.

@WanruWu-NOAA WanruWu-NOAA deleted the wwu-cfe2 branch March 10, 2023 19:24
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