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

Remove mpas_model from general.config.ocean #20

Closed
mark-petersen opened this issue Nov 9, 2020 · 7 comments · Fixed by #28
Closed

Remove mpas_model from general.config.ocean #20

mark-petersen opened this issue Nov 9, 2020 · 7 comments · Fixed by #28
Labels
bug Something isn't working

Comments

@mark-petersen
Copy link
Collaborator

This line really means the compass path now, not mpas_model:

general.config.ocean:39:
  mpas_model = FULL_PATH_TO_MPAS_MODEL_REPO

But it is only used in two places:

ocean/soma/32km/default/config_analysis.xml:3:  
  <add_link source_path="mpas_model" source="testing_and_setup/compass/ocean/soma/32km/default/analysis/check_particle_sampling.py" dest="check_particle_sampling.py"/>
ocean/ziso/20km/default/config_forward.xml:8:
  <add_link source_path="mpas_model" source="testing_and_setup/compass/ocean/scripts/LIGHTparticles/make_particle_file.py" dest="make_particles.py"/>

so I think we can just get rid of that config flag completely, as we should be able to specify those paths in a different way. Also, the testing_and_setup/compass is incorrect now anyway.

@mark-petersen mark-petersen added the bug Something isn't working label Nov 9, 2020
@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

@mark-petersen, I'm happy to do this, but I kind of think of things the other way around. With config files with extended interpolation, it makes sense to specify only the mpas_model path and have the config file (at least by default) figure everything else out from that path. Here's an example from what I'm working on:

# This config file has default config options for the ocean core

# The paths section points COMPASS to external paths
[paths]

# the relative or absolute path to the root of a branch where MPAS-Ocean
# has been built
mpas_model = MPAS-Model/ocean/develop

# The namelists section defines paths to template_compact namelists that will be used
# to generate specific namelists. By default, these point to the forward and
# init namelists in the default_inputs directory after a successful build of
# the ocean model.  Change these in a custom config file if you need a different
# template_compact.
[namelists]
forward = ${paths:mpas_model}/default_inputs/namelist.ocean.forward
init    = ${paths:mpas_model}/default_inputs/namelist.ocean.init

# The streams section defines paths to template_compact streams files that will be used
# to generate specific streams files. By default, these point to the forward and
# init streams files in the default_inputs directory after a successful build of
# the ocean model. Change these in a custom config file if you need a different
# template_compact.
[streams]
forward = ${paths:mpas_model}/default_inputs/streams.ocean.forward
init    = ${paths:mpas_model}/default_inputs/streams.ocean.init


# The executables section defines paths to required executables. These
# executables are provided for use by specific test cases.  Most tools that
# COMPASS needs should be in the conda environment, so this is only the path
# to the MPAS-Ocean executable by default.
[executables]
model = ${paths:mpas_model}/ocean_model

@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

But I agree that the two cases you point out above should me altered to not use those silly paths anymore.

@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

I'll make a PR to fix these 2.

@mark-petersen
Copy link
Collaborator Author

Yes, your example config file is much better. I've never specified different paths for any of those things, and made a sed script a long time ago to replace them all at once.

Your example above also removes

 40 mesh_database = FULL_PATH_TO_LOCAL_MESH_DATABASE
 41 initial_condition_database = FULL_PATH_TO_LOCAL_INITIAL_CONDITION_DATABASE
 42 bathymetry_database = FULL_PATH_TO_BATHYMETRY_DATABASE

You don't mean to remove those, right? It is nice to point to files that are in-place on our shared machines.

@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

Yes, I use sed for this, too. And this won't work with the ConfigParser as currently used in COMPASS, but it will work in the future version I'm working on.

@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

@mark-petersen, I have a branch for this but I'm having trouble testing it because the LCRC server is down (monthly maintenance?) so I can't get mesh files I need.

@xylar
Copy link
Collaborator

xylar commented Nov 9, 2020

Your example above also removes ...

Didn't respond to this. I didn't mean for my example to be a complete one. The way things will work, those paths will come from a machine-specific config file, which is separate from the config file where you specify the location of MPAS-Ocean. But I'm getting ahead of myself. I just wanted to point out that there may be a good reason to keep mpas_model as a path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants