search

ROSS-org / ROSS

Rensselaer's Optimistic Simulation System
http://ross-org.github.io
BSD 3-Clause "New" or "Revised" License
92 stars 46 forks source link

Formalize ROSS API #162

Open gonsie opened 4 years ago

gonsie commented 4 years ago

The ROSS API isn't very well documented. Models are expected to manually set several g_tw_xx variables, all of which are exposed through the header files... which means that an installation of ROSS must install several header files.

In my dream world, only ross.h is installed and the API to configure ROSS is consistent and clearly documented.

Thoughts from @markplagge and @nmcglohon (or others from codes-org)?

I think the way to make progress on this would be:

  1. figure out which global variables models are using.
  2. create an API to allow models to set various options.
  3. clean up ross.h. Maybe create a new ross-internal.h for ross itself and use ross.h as the one for models to include?
caitlinross commented 4 years ago

Yeah I think this would be a good idea. It would also be useful for making improvements in ROSS without affecting models. e.g., when removing the remnants of pthread ROSS, g_tw_pe used to be an array of PE pointers, so I changed it. Though of course that required a change in CODES because in their mapping set up, they accessed that PE array. We should be able to make changes in ROSS like that without being able to affect models.

On that note, I know for sure that g_tw_pe is used in CODES. :) And g_tw_ts_end. I'm sure there's more but I know those off the top of my head.

markplagge commented 4 years ago

I think this would be a great idea. Having a single ross.h file to include which contains the components that models should be using is a great idea. Making ROSS more stand-alone library like is a good thing.

Moving to something like this is pretty challenging, especially without breaking backwards compatibility. I know that NEMO uses the global LP array, but that is being removed. The new code-base for NeMo actually uses the tw_pe function hooks too.

I like number 3, that would make things simpler as it would be obvious which ROSS functions are okay to use, and which would be risky for a model to use.

nmcglo commented 4 years ago

Sounds good to me, I'm a big fan of abstracting away stuff like that. That way future changes don't require such broad changes across CODES.

My only request would be to have some warning before it's merged to master. Keeping CODES master always compatible with latest ROSS master is my goal as when that doesn't work I start to get emails :joy: so synchronizing an API change would be preferable for me.

This all being said, CODES uses gtw* values A TON. About 277 times. A lot of g_tw_ts_end, g_tw_lookahead, g_tw_mynode is the majority. I can make an exhaustive list though.