Skip to content

targeting 9c96d17 MOAB commit instead of version 5.1#740

Merged
gonuke merged 7 commits into
svalinn:developfrom
bam241:moab_update
Jun 17, 2021
Merged

targeting 9c96d17 MOAB commit instead of version 5.1#740
gonuke merged 7 commits into
svalinn:developfrom
bam241:moab_update

Conversation

@bam241

@bam241 bam241 commented Jun 15, 2021

Copy link
Copy Markdown
Member

This is a temporary MOAB update in CI to allow our CI to pass.

this will be change at the next MOAB release

I am building the docker stack right now, I'll push them on request

@bam241 bam241 requested review from gonuke and pshriwise June 15, 2021 15:31
@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

@gonuke @pshriwise docker build stack is finally built.
let me know what you want to do from there.

@gonuke

gonuke commented Jun 16, 2021

Copy link
Copy Markdown
Member

Can you test locally on the new docker stack?

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if there is a way to structure these files to define a DAGMC_CI_MOAB_VERSION in one place and then refer to it throughout??

@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

I wonder if there is a way to structure these files to define a DAGMC_CI_MOAB_VERSION in one place and then refer to it throughout??

I'll try to think of a way... the problem is that is not probably easy to have bash talking to Circle.... (.circle.yml had to be changed as well....)

In the mean time I'll build and test DAGMC the 4 docker images built against MOAB 9c96d17

@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

successfully tested:

  • svalinn/dagmc-ci-ubuntu-18.04-clang-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-18.04-gcc-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-16.04-clang-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-16.04-gcc-ext-hdf5_1.10.4-moab_9c96d17

@gonuke

gonuke commented Jun 16, 2021

Copy link
Copy Markdown
Member

I wonder if there is a way to structure these files to define a DAGMC_CI_MOAB_VERSION in one place and then refer to it throughout??

I'll try to think of a way... the problem is that is not probably easy to have bash talking to Circle.... (.circle.yml had to be changed as well....)

In the mean time I'll build and test DAGMC the 4 docker images built against MOAB 9c96d17

A first step might be to use the full branch name Version5.1.0 under normal circumstances. Then checking out master vs develop vs some other branch don't require different logic and name mangling.

@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

Would you like me to implement this here ?
Or think it though and implement this when we switch back to a branch version of MOAB
(it might not make a lot of sense yet as we are building against a specific commit....)

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A couple of optional thoughts on the way to streamlining this change

Comment thread CI/Dockerfile_3_moab
# MOAB Verions: 5.1.0
ARG MOAB=5.1.0
# MOAB Commit: 9c96d17 (Merged commit of @pshriwise thread fix)
ARG MOAB=9c96d17

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

doesn't this get overridden by the env variable when called via update_docker.sh. Therefore, this could keep 5.1.0 as the default, but the value is changed in the script?

Comment thread .circleci/config.yml
compiler: ["clang", "gcc"]
hdf5: ["1.10.4"]
moab: ["5.1.0"]
moab: ["9c96d17"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know enough about CI/yaml to know if this can be defined only once in this file and reused???

@gonuke

gonuke commented Jun 16, 2021

Copy link
Copy Markdown
Member

Would you like me to implement this here ?
Or think it though and implement this when we switch back to a branch version of MOAB
(it might not make a lot of sense yet as we are building against a specific commit....)

See the two small comments I just made and if you think they make sense, we can stop there and work to merge this.

@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

Would you like me to implement this here ?
Or think it though and implement this when we switch back to a branch version of MOAB
(it might not make a lot of sense yet as we are building against a specific commit....)

See the two small comments I just made and if you think they make sense, we can stop there and work to merge this.

I think they make sense, but I'd like to do it cleanly all the way, instead of patching the file one after the other...

I'll prepare a dedicated PR to do this!

@gonuke

gonuke commented Jun 16, 2021

Copy link
Copy Markdown
Member

successfully tested:

  • svalinn/dagmc-ci-ubuntu-18.04-clang-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-18.04-gcc-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-16.04-clang-ext-hdf5_1.10.4-moab_9c96d17
  • svalinn/dagmc-ci-ubuntu-16.04-gcc-ext-hdf5_1.10.4-moab_9c96d17

I think these are all the tests, right? So then we can merge this and push those images to dockerhub?

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No comments from me. Thanks for addressing this @bam241!

@gonuke

gonuke commented Jun 16, 2021

Copy link
Copy Markdown
Member

Do you want to push these images first and then relaunch tests?

@bam241

bam241 commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

sure will do!

@bam241

bam241 commented Jun 17, 2021

Copy link
Copy Markdown
Member Author

@pshriwise @gonuke docker stack is updated all tests are running fine

@gonuke

gonuke commented Jun 17, 2021

Copy link
Copy Markdown
Member

Thanks @bam241

@gonuke gonuke merged commit 3401ad9 into svalinn:develop Jun 17, 2021
@bam241 bam241 deleted the moab_update branch November 28, 2024 14:40
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.

3 participants