IBAMR / IBSAMRAI2

SAMRAI 2.4.4 fork and associated patches.
Other
1 stars 1 forks source link

Shall we make this repository public? #17

Closed drwells closed 7 months ago

drwells commented 8 months ago

I think it's time we just offer our own patched SAMRAI downloads instead of relying on LLNL + requiring users to patch the result. Hence, I propose we do the following:

  1. Make this repository public.
  2. Increment the version number here to 2.4.5 to disambiguate between our old patch and our new patch.
  3. Update the installation instructions to refer to releases made here instead of via LLNL.

It would be best to move to SAMRAI 4 etc. but we are de facto running our own fork via patches anyway (and SAMRAI 2.4.x doesn't even compile with out our patches) and this is backwards compatible with all IBAMR apps.

drwells commented 8 months ago

@amneetb I just added you to this repository - I'm not sure if we talked about it but this is how Boyce and I have been collecting other SAMRAI bug fixes / parallel performance improvements. It is 100% backwards compatible.

amneetb commented 8 months ago

Making this public sounds good to me. Hopefully there are no license issues here.

drwells commented 8 months ago

Good question - I see

Commercialization of this product is prohibited without notifying the
Department of Energy (DOE) or Lawrence Livermore National Laboratory
(LLNL).

in the top-level copyright statement, which isn't exactly a software license (like BSD). Circa 2011 SAMRAI formally adopted the LGPL. Hence, if we want to be technical, it's reasonable to assume that the authors agreed to license a patched version (i.e., changes from 2008 - 2011) of this software under that license. That license definitely conflicts with that copyright statement. I think it's fair to say that SAMRAI 2.4.4 is available with no license except 'tell LLNL if you sell it'.

boyceg commented 8 months ago

I am not sure about calling this SAMRAI 2.4.5. I would suggest that we call it something else (ibSAMRAI2?) that makes it clear that it is related to SAMRAI 2 but somehow a fork and adopt release date-based version numbers.

drwells commented 8 months ago

We could rename the project as a whole but we really need to keep namespace SAMRAI:: etc. everywhere so that people's projects, makefiles, etc. are not broken. In general, I'm fine with renaming as long as everything still compiles.

boyceg commented 8 months ago

Yeah I think that's fine. Could we change the namespaces but provide an alias?

drwells commented 8 months ago

What would be the primary advantage of adding a new namespace? One disadvantage I can think of is that the doxygen documentation will look weirder.

boyceg commented 8 months ago

I don't know --- you were the one who mentioned namespaces!! 😁

drwells commented 8 months ago

Did I? I shouldn't have because I don't want to mess up doxygen (or to introduce weird link errors when people use forward declarations).

drwells commented 8 months ago

I think we need to come to a consensus on this prior to the release. We need to

  1. stay binary compatible - I don't want to introduce alias namespaces. Things should still be in SAMRAI::tbox
  2. stay API compatible - things should work with older versions of SAMRAI
  3. fix compilation problems, performance bugs, and correctness bugs
  4. stay license-compatible (see above)
  5. somehow distinguish ourselves from vanilla samrai 2.4.4

Hence I still think our best option is to keep the name as-is and increment the patch number to 2.4.5.

I want to get this done this week so if there's an alternative to achieving all of those goals lets hear it soon.

boyceg commented 8 months ago

My suggestion is as above --- let's name it something like ibSAMRAI2 and set the version number to the release date.

boyceg commented 8 months ago

(But keep the namespacing SAMRAI!)

drwells commented 8 months ago

so re-name this repository?

boyceg commented 8 months ago

That is my vote.

abarret commented 8 months ago

I second renaming the repository. I think it's important to distinguish this repo from the main SAMRAI repo.

drwells commented 8 months ago

Sounds good - lets do IBSAMRAI2 (mixing cases is too weird, lets make it all capitals).

For version numbers: we currently do

SET(SAMRAI_VERSION_STRING "${SAMRAI_VERSION_MAJOR}.${SAMRAI_VERSION_MINOR}\
.${SAMRAI_VERSION_PATCHLEVEL}")
MESSAGE(STATUS "Found SAMRAI ${SAMRAI_VERSION_STRING} at ${SAMRAI_ROOT}")

and I'd like to keep a similar approach to that to maintain compatibility with existing installations. Is your proposal that we do 2024.01.23?

drwells commented 7 months ago

I think we're in agreement so I'm going to roll with it. Rename and release incoming.