search

ECP-CANDLE / candle_lib

MIT License
1 stars 9 forks source link

Update setup.py #28

Open rajeeja opened 1 year ago

rajeeja commented 1 year ago

Remove specific version dependency on protobuf, the old problems seems to be fixed in newer versions of protobuf. Tested on OSX.

j-woz commented 1 year ago

This works for me.

rajeeja commented 1 year ago

@jmohdyusof merge?

jmohdyusof commented 1 year ago

You need to test actual benchmarks. Almost all the benchmarks in Pilot1 and Pilot3 fail with: TypeError: Descriptors cannot not be created directly. If this call came from a _pb2.py file, your generated code is out of date and must be regenerated with protoc >= 3.19.0. If you cannot immediately regenerate your protos, some other possible workarounds are:

  1. Downgrade the protobuf package to 3.20.x or lower.
  2. Set PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python (but this will use pure-Python parsing and will be much slower).

All the data files need to recreated with newer protobuf, based on that error message.

rajeeja commented 1 year ago

All the data files need to recreated with newer protobuf, based on that error message. How to resolve dependency issues? it is a nighmare to reolve dependency issues on new HPC machines

Option 1: I think we should coordinate with model owners and discuss the situation with them. Request that they provide the scripts or tools they used to generate the data files. This way, you can recreate the data files using the newer version of protobuf while ensuring compatibility with the benchmarks. This is a hard option, but, doable.

Option 2: Create a separate branch: If it's not feasible to update the benchmarks immediately, you can create a separate branch in the candle_lib repository specifically for supporting the benchmarks in the Benchmarks repository. This branch, let's call it benchmarks_compatible, can be dedicated to maintaining compatibility with the older version of protobuf required by the benchmarks. This approach allows you to freeze the necessary codebase for the benchmarks while allowing the master branch to move forward with updates and improvements.

Another key issue: Plan for a new release: @brettin what is the timeline to determine a timeline for a new release of candle_lib? This new release should incorporate updates, bug fixes, and improvements while considering compatibility requirements with the benchmarks. Discuss the possibility of diverging from the notion of the master branch supporting all benchmarks and confining it to the benchmarks_compatible branch.

What ever route we follow, we should communicate the plan, decisions, and any necessary changes to all relevant stakeholders. This documentation will help maintain clarity and facilitate future development and collaboration.

jmohdyusof commented 1 year ago

Well, a lot of these are 'your' benchmarks, so you can update them is you wish. All the Pilot1 benchmarks fail

rajeeja commented 1 year ago

Well, a lot of these are 'your' benchmarks, so you can update them is you wish. All the Pilot1 benchmarks fail

I didn't write the benchmarks and don't have the scripts. Let's go with Option2.

jmohdyusof commented 1 year ago

What do you mean 'I don't have the scripts'? They are all there in Pilot 1

rajeeja commented 1 year ago

What do you mean 'I don't have the scripts'? They are all there in Pilot 1

scripts that were used to generate the input data files.

jmohdyusof commented 1 year ago

I assume Tom, Fangfang, Yitan must have these? I expect if they are rerun with a newer protobuf environment they would work?