MindTheGap-ERC / CarboKitten.jl

Julia implementation of carbonate platform model
https://mindthegap-erc.github.io/CarboKitten.jl/
GNU General Public License v3.0
0 stars 0 forks source link

ERROR: UndefVarError: `BS92` not defined #28

Closed HannoSpreeuw closed 3 months ago

HannoSpreeuw commented 4 months ago

Following the instruction from the README:

include("examples/bosscher-schlager-1992.jl")

from the Julia REPL gives

ERROR: UndefVarError: `BS92` not defined
Stacktrace:
 [1] include(fname::String)
   @ Base.MainInclude ./client.jl:489
 [2] top-level scope
   @ REPL[1]:1
HannoSpreeuw commented 4 months ago

Suggested solution (@xyl96 and @NiklasHohmann) remove from README and keep in xy_test branch.

EmiliaJarochowska commented 3 months ago

I don't get this solution. BS92.jl is also disabled in xy_test and I don't see it use this example anywhere. And this branch is supposed to be merged soon (hopefully).

include("examples/bosscher-schlager-1992.jl") in the README is a relict from the times when CarboKitten didn't give any output yet. It should be now replaced with an example of the core functionality of CarboKitten, such as ca-with-prod.jl. This example produces an output file but not a plot. Because CarboKitten.Visualization has also been disabled, most other examples won't work. It is not clear to me what is supposed to happen with it. Is it supposed to become a separate module?

examples/bosscher-schlager-1992.jl is really neat to understand the principles of the model but it isn't, strictly speaking, CarboKitten. It is a separate piece of code. Is the focus in this issue on updating the README or on the role of BS92.jl in the module, @HannoSpreeuw ?

HannoSpreeuw commented 3 months ago

@EmiliaJarochowska Updating the README; all the instructions from the README should execute flawlessly and produce the expected output.

@xyl96 and @NiklasHohmann Why do you want to keep BS92.jl in the xy_test branch? You must have told me, but I forgot.

EmiliaJarochowska commented 3 months ago

Then I would swap this for one of the examples that work and make a separate issue on BS92 and visualization. Do you want me to do it?

HannoSpreeuw commented 3 months ago

Fair enough. Which example would you like to swap with? I can implement that and close this issue.

Please add new issues on BS92 and/or visualization, if necessary.

EmiliaJarochowska commented 3 months ago

I have some other corrections to the README so let me branch for a sec and you can review my edits

EmiliaJarochowska commented 3 months ago

Please add new issues on BS92 and/or visualization, if necessary.

Re visualization, we have an issue that addresses this to some extent: #17

@jhidding wrote in this issue:

Created a separate workenv where all the big dependencies for interactive use go. Now CarboKitten itself only depends on HDF5 and Transducers.

One snatch: if you work in a different branch, you need to tell workenv that you want to use that branch of CarboKitten.

I think this is supposed to be the ca-with-production branch so when you switch to this branch in VS Code, its environment with all the dependencies is loaded automatically (could be also by hand I guess). But that's not clear in this instruction so it needs to be completed.

Secondly, I don't know if this good in terms of user experience. And even developer experience. You need to remember to keep this branch up to date (which it isn't, but am I looking at the right one?) and it requires users comfortable with git and Julia's environments. Wouldn't making Visualization a separate module be neater?

jhidding commented 3 months ago

Once we release, things would be more stable. Only when someone starts to develop in a separate branch they need to know what they're doing. Indeed creating a separate visualization package is probably the way to go, to make it easier to work from a fresh Julia install.